[Lldb-commits] [PATCH] D64251: Don't depend on psutil on AIX

[Dan Liew via Phabricator via lldb-commits]

delcypher added inline comments.



Comment at: llvm/utils/lit/lit/LitConfig.py:81
+@property
+def killProcessAndChildrenIsSupported(self):
+"""

Sorry to be pedantic but this (`LitConfig.killProcessAndChildrenIsSupported`) 
should be called `maxIndividualTestTimeIsSupported` to be consistent with the 
rest of `LitConfig`'s public interface. The name 
`killProcessAndChildrenIsSupported` is leaking implementation details.



Comment at: llvm/utils/lit/lit/util.py:426
 
+def killProcessAndChildrenIsSupported(llvm_config):
+"""

I don't really like how we're now coupling this function with the `LitConfig` 
object just so we can print out the text `Found python psutil module`. Do we 
actually need to print out that message? If the tests don't rely on this I 
either suggest we remove this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64251



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

[Lldb-commits] [PATCH] D64251: Don't depend on psutil on AIX

[David Tenty via Phabricator via lldb-commits]

daltenty updated this revision to Diff 209409.
daltenty added a comment.

- Address review comments round 2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64251

Files:
  libcxx/utils/libcxx/util.py
  lldb/lit/lit.cfg.py
  llvm/utils/lit/lit/LitConfig.py
  llvm/utils/lit/lit/util.py
  llvm/utils/lit/tests/googletest-timeout.py
  llvm/utils/lit/tests/lit.cfg
  llvm/utils/lit/tests/shtest-timeout.py

Index: llvm/utils/lit/tests/shtest-timeout.py
===
--- llvm/utils/lit/tests/shtest-timeout.py
+++ llvm/utils/lit/tests/shtest-timeout.py
@@ -1,4 +1,4 @@
-# REQUIRES: python-psutil
+# REQUIRES: lit-max-individual-test-time
 
 # llvm.org/PR33944
 # UNSUPPORTED: system-windows
Index: llvm/utils/lit/tests/lit.cfg
===
--- llvm/utils/lit/tests/lit.cfg
+++ llvm/utils/lit/tests/lit.cfg
@@ -1,6 +1,7 @@
 # -*- Python -*-
 
 import os
+import platform
 import sys
 
 import lit.formats
@@ -56,10 +57,10 @@
 os.path.dirname(__file__), ".coveragerc")
 
 # Add a feature to detect if psutil is available
-try:
-import psutil
-lit_config.note('Found python psutil module')
-config.available_features.add("python-psutil")
-except ImportError:
-lit_config.warning('Could not import psutil. Some tests will be skipped and'
-   ' the --timeout command line argument will not work.')
+supported, errormsg = lit_config.killProcessAndChildrenIsSupported
+if supported:
+config.available_features.add("lit-max-individual-test-time")
+else:
+lit_config.warning('Setting a timeout per test not supported. ' + errormsg
+   + ' Some tests will be skipped and the --timeout'
+ ' command line argument will not work.')
Index: llvm/utils/lit/tests/googletest-timeout.py
===
--- llvm/utils/lit/tests/googletest-timeout.py
+++ llvm/utils/lit/tests/googletest-timeout.py
@@ -1,4 +1,4 @@
-# REQUIRES: python-psutil
+# REQUIRES: lit-max-individual-test-time
 
 # Check that the per test timeout is enforced when running GTest tests.
 #
Index: llvm/utils/lit/lit/util.py
===
--- llvm/utils/lit/lit/util.py
+++ llvm/utils/lit/lit/util.py
@@ -423,6 +423,26 @@
 return out
 return None
 
+def killProcessAndChildrenIsSupported(llvm_config):
+"""
+Returns a tuple ( , )
+where
+`` is True if `killProcessAndChildren()` is supported on
+the current host, returns False otherwise.
+`` is an empty string if `` is True,
+otherwise is contains a string describing why the function is
+not supported.
+"""
+if platform.system() == 'AIX':
+return (True, "")
+try:
+import psutil  # noqa: F401
+llvm_config.note('Found python psutil module')
+return (True, "")
+except ImportError:
+return (False,  "Requires the Python psutil module but it could"
+" not be found. Try installing it via pip or via"
+" your operating system's package manager.")
 
 def killProcessAndChildren(pid):
 """This function kills a process with ``pid`` and all its running children
@@ -433,24 +453,27 @@
 our dependency on it.
 
 """
-import psutil
-try:
-psutilProc = psutil.Process(pid)
-# Handle the different psutil API versions
+if platform.system() == 'AIX':
+subprocess.call('kill -kill $(ps -o pid= -L{})'.format(pid), shell=True)
+else:
+import psutil
 try:
-# psutil >= 2.x
-children_iterator = psutilProc.children(recursive=True)
-except AttributeError:
-# psutil 1.x
-children_iterator = psutilProc.get_children(recursive=True)
-for child in children_iterator:
+psutilProc = psutil.Process(pid)
+# Handle the different psutil API versions
 try:
-child.kill()
-except psutil.NoSuchProcess:
-pass
-psutilProc.kill()
-except psutil.NoSuchProcess:
-pass
+# psutil >= 2.x
+children_iterator = psutilProc.children(recursive=True)
+except AttributeError:
+# psutil 1.x
+children_iterator = psutilProc.get_children(recursive=True)
+for child in children_iterator:
+try:
+child.kill()
+except psutil.NoSuchProcess:
+pass
+psutilProc.kill()
+except psutil.NoSuchProcess:
+pass
 
 
 try:
Index: llvm/utils/lit/lit/LitConfig.py
===
--- 

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365853: [Expression] Move IRDynamicChecks to 
ClangExpressionParser (authored by xiaobai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64591?vs=209351=209388#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64591

Files:
  lldb/trunk/include/lldb/Expression/DynamicCheckerFunctions.h
  lldb/trunk/include/lldb/Expression/IRDynamicChecks.h
  lldb/trunk/source/Expression/CMakeLists.txt
  lldb/trunk/source/Expression/IRDynamicChecks.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
  lldb/trunk/source/Target/Process.cpp
  lldb/trunk/source/Target/ThreadPlanCallUserExpression.cpp

Index: lldb/trunk/source/Target/ThreadPlanCallUserExpression.cpp
===
--- lldb/trunk/source/Target/ThreadPlanCallUserExpression.cpp
+++ lldb/trunk/source/Target/ThreadPlanCallUserExpression.cpp
@@ -13,7 +13,7 @@
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Target/LanguageRuntime.h"
Index: lldb/trunk/source/Target/Process.cpp
===
--- lldb/trunk/source/Target/Process.cpp
+++ lldb/trunk/source/Target/Process.cpp
@@ -22,7 +22,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
Index: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -62,6 +62,7 @@
 #include "ClangHost.h"
 #include "ClangModulesDeclVendor.h"
 #include "ClangPersistentVariables.h"
+#include "IRDynamicChecks.h"
 #include "IRForTarget.h"
 #include "ModuleDependencyCollector.h"
 
@@ -69,7 +70,6 @@
 #include "lldb/Core/Disassembler.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/StreamFile.h"
-#include "lldb/Expression/IRDynamicChecks.h"
 #include "lldb/Expression/IRExecutionUnit.h"
 #include "lldb/Expression/IRInterpreter.h"
 #include "lldb/Host/File.h"
@@ -1281,8 +1281,8 @@
 (execution_policy != eExecutionPolicyTopLevel && !can_interpret)) {
   if (m_expr.NeedsValidation() && process) {
 if (!process->GetDynamicCheckers()) {
-  DynamicCheckerFunctions *dynamic_checkers =
-  new DynamicCheckerFunctions();
+  ClangDynamicCheckerFunctions *dynamic_checkers =
+  new ClangDynamicCheckerFunctions();
 
   DiagnosticManager install_diagnostics;
 
@@ -1302,23 +1302,26 @@
 "Finished installing dynamic checkers ==");
 }
 
-IRDynamicChecks ir_dynamic_checks(*process->GetDynamicCheckers(),
-  function_name.AsCString());
-
-llvm::Module *module = execution_unit_sp->GetModule();
-if (!module || !ir_dynamic_checks.runOnModule(*module)) {
-  err.SetErrorToGenericError();
-  err.SetErrorString("Couldn't add dynamic checks to the expression");
-  return err;
-}
+if (auto *checker_funcs = llvm::dyn_cast(
+process->GetDynamicCheckers())) {
+  IRDynamicChecks ir_dynamic_checks(*checker_funcs,
+function_name.AsCString());
+
+  llvm::Module *module = execution_unit_sp->GetModule();
+  if (!module || !ir_dynamic_checks.runOnModule(*module)) {
+err.SetErrorToGenericError();
+err.SetErrorString("Couldn't add dynamic checks to the expression");
+return err;
+  }
 
-if (custom_passes.LatePasses) {
-  if (log)
-log->Printf("%s - Running Late IR Passes from LanguageRuntime on "
-"expression module '%s'",
-__FUNCTION__, m_expr.FunctionName());
+  if (custom_passes.LatePasses) {
+if (log)
+  log->Printf("%s - Running Late IR 

[Lldb-commits] [lldb] r365853 - [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Thu Jul 11 17:58:02 2019
New Revision: 365853

URL: http://llvm.org/viewvc/llvm-project?rev=365853=rev
Log:
[Expression] Move IRDynamicChecks to ClangExpressionParser

Summary:
IRDynamicChecks in its current form is specific to Clang since it deals
with the C language family. It is possible that we may want to
instrument code generated for other languages, but we can factor in a
more general mechanism to do so at a later time.

This decouples ObCLanguageRuntime from Expression!

Reviewers: compnerd, clayborg, jingham, JDevlieghere

Subscribers: mgorny, lldb-commits

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

Added:
lldb/trunk/include/lldb/Expression/DynamicCheckerFunctions.h
lldb/trunk/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
  - copied, changed from r365843, 
lldb/trunk/source/Expression/IRDynamicChecks.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
  - copied, changed from r365843, 
lldb/trunk/include/lldb/Expression/IRDynamicChecks.h
Removed:
lldb/trunk/include/lldb/Expression/IRDynamicChecks.h
lldb/trunk/source/Expression/IRDynamicChecks.cpp
Modified:
lldb/trunk/source/Expression/CMakeLists.txt
lldb/trunk/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/trunk/source/Target/Process.cpp
lldb/trunk/source/Target/ThreadPlanCallUserExpression.cpp

Added: lldb/trunk/include/lldb/Expression/DynamicCheckerFunctions.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/DynamicCheckerFunctions.h?rev=365853=auto
==
--- lldb/trunk/include/lldb/Expression/DynamicCheckerFunctions.h (added)
+++ lldb/trunk/include/lldb/Expression/DynamicCheckerFunctions.h Thu Jul 11 
17:58:02 2019
@@ -0,0 +1,62 @@
+//===-- DynamicCheckerFunctions.h ---*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef liblldb_DynamicCheckerFunctions_h_
+#define liblldb_DynamicCheckerFunctions_h_
+
+#include "lldb/lldb-types.h"
+
+namespace lldb_private {
+
+class DiagnosticManager;
+class ExecutionContext;
+
+/// Encapsulates dynamic check functions used by expressions.
+///
+/// Each of the utility functions encapsulated in this class is responsible
+/// for validating some data that an expression is about to use.  Examples
+/// are:
+///
+/// a = *b; // check that b is a valid pointer
+/// [b init];   // check that b is a valid object to send "init" to
+///
+/// The class installs each checker function into the target process and makes
+/// it available to IRDynamicChecks to use.
+class DynamicCheckerFunctions {
+public:
+  enum DynamicCheckerFunctionsKind {
+DCF_Clang,
+  };
+
+  DynamicCheckerFunctions(DynamicCheckerFunctionsKind kind) : m_kind(kind) {}
+  virtual ~DynamicCheckerFunctions() = default;
+
+  /// Install the utility functions into a process.  This binds the instance
+  /// of DynamicCheckerFunctions to that process.
+  ///
+  /// \param[in] diagnostic_manager
+  /// A diagnostic manager to report errors to.
+  ///
+  /// \param[in] exe_ctx
+  /// The execution context to install the functions into.
+  ///
+  /// \return
+  /// True on success; false on failure, or if the functions have
+  /// already been installed.
+  virtual bool Install(DiagnosticManager _manager,
+   ExecutionContext _ctx) = 0;
+  virtual bool DoCheckersExplainStop(lldb::addr_t addr, Stream ) = 0;
+
+  DynamicCheckerFunctionsKind GetKind() const { return m_kind; }
+
+private:
+  const DynamicCheckerFunctionsKind m_kind;
+};
+} // namespace lldb_private
+
+#endif // liblldb_DynamicCheckerFunctions_h_

Removed: lldb/trunk/include/lldb/Expression/IRDynamicChecks.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/IRDynamicChecks.h?rev=365852=auto
==
--- lldb/trunk/include/lldb/Expression/IRDynamicChecks.h (original)
+++ lldb/trunk/include/lldb/Expression/IRDynamicChecks.h (removed)
@@ -1,145 +0,0 @@
-//===-- IRDynamicChecks.h ---*- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef liblldb_IRDynamicChecks_h_
-#define liblldb_IRDynamicChecks_h_
-
-#include "lldb/lldb-types.h"
-#include "llvm/Pass.h"
-
-namespace llvm {
-class 

[Lldb-commits] [PATCH] D64194: [lldb] Fix crash due to unicode characters and dollars in variable names.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor accepted this revision.
teemperor added a comment.

Closed by 365698


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

https://reviews.llvm.org/D64194



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D64591#1581878 , @jingham wrote:

> That would be cleaner.
>
> OTOH, the original reason for these checkers was to help people understand 
> crashes in their expressions more clearly.  Supposedly, modern languages 
> "don't have pointers" and can't have bad objects, so the kind of crashes this 
> instrumentation was supposed to help with "can't happen" and checkers for 
> such languages wouldn't be all that helpful...
>
> So while cleaner, maybe generalizing this more fully isn't a high priority 
> change?  In which case, just getting them out of generic code seems fine as a 
> stopping point.  Your choice.


I don't think of it as high priority. That might change at some in the future, 
but for the time being I think that there are bigger fish to fry.


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [lldb] r365843 - [Target] Replace Plugin headers with non-plugin headers

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Thu Jul 11 16:45:35 2019
New Revision: 365843

URL: http://llvm.org/viewvc/llvm-project?rev=365843=rev
Log:
[Target] Replace Plugin headers with non-plugin headers

Modified:
lldb/trunk/source/Target/Target.cpp

Modified: lldb/trunk/source/Target/Target.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=365843=365842=365843=diff
==
--- lldb/trunk/source/Target/Target.cpp (original)
+++ lldb/trunk/source/Target/Target.cpp Thu Jul 11 16:45:35 2019
@@ -7,9 +7,7 @@
 
//===--===//
 
 #include "lldb/Target/Target.h"
-#include "Plugins/ExpressionParser/Clang/ClangASTSource.h"
 #include "Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.h"
-#include "Plugins/ExpressionParser/Clang/ClangPersistentVariables.h"
 #include "lldb/Breakpoint/BreakpointIDList.h"
 #include "lldb/Breakpoint/BreakpointPrecondition.h"
 #include "lldb/Breakpoint/BreakpointResolver.h"
@@ -39,6 +37,7 @@
 #include "lldb/Interpreter/OptionValues.h"
 #include "lldb/Interpreter/Property.h"
 #include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol/ClangASTImporter.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/Symbol.h"


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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Jim Ingham via Phabricator via lldb-commits]

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

That would be cleaner.

OTOH, the original reason for these checkers was to help people understand 
crashes in their expressions more clearly.  Supposedly, modern languages "don't 
have pointers" and can't have bad objects, so the kind of crashes this 
instrumentation was supposed to help with "can't happen" and checkers for such 
languages wouldn't be all that helpful...

So while cleaner, maybe generalizing this more fully isn't a high priority 
change?  In which case, just getting them out of generic code seems fine as a 
stopping point.  Your choice.


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D64591#1581820 , @jingham wrote:

> Then the IRDynamicCheck part would go with LLVMUserExpression, and the 
> C-specific checks in Clang...


Except IRDynamicChecks has special knowledge of the clang-specific 
instrumenters. I can think of another way in which DynamicCheckerFunctions and 
IRDynamicChecks are more generic classes that will work with any language. I 
can try to explore that option to see what I come up with, maybe the end result 
will be better?


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Then the IRDynamicCheck part would go with LLVMUserExpression, and the 
C-specific checks in Clang...


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin

[Jim Ingham via Phabricator via lldb-commits]

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

Okay...  Provided we can come up with not too torturous ways to get the ObjC 
and Swift support out of generic-code, it seems okay to do this as a first 
step.  I just don't want to end up in the state where we have 
ObjCLanguageRuntime as generic but CPPLanguageRuntime is not.


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

https://reviews.llvm.org/D64599



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D64591#1581775 , @jingham wrote:

> This is a little unclear to me.  LLVMUserExpression is the class that 
> represents "anything that uses LLVM at its back end."  Both the Swift & Clang 
> user expressions are instances of this.  Running through IR and inserting 
> little callouts is an LLVMUserExpression type thing.  So it seems like this 
> move is in the right direction, but to be really clean needs to represent 
> LLVMUserExpression in the Plugin Hierarchy.


Right, this makes sense to me. My understanding was that LLVMUserExpression 
would eventually invoke ClangExpressionParser, which actually handles dealing 
with IR instrumentation. With swift in the picture, things get a little more 
complicated because you could want to deal with both ObjC code and Swift code. 
I think that in that case, LLDB would want to use both Clang and Swift to 
instrument a user expression. I don't think that the abstractions are set up to 
account for this use case right now, so it would require some planning and 
refactoring.


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D64599#1581718 , @jingham wrote:

> Anyway, if you can make all the generic parts of lldb not depend on the 
> language specific - but still abstract - part of the plugin, that would be 
> fine.  Then just LanguageRuntime.h would live in Target, and 
> CPPLanguageRuntime would live in Plugins/Language


This is exactly where I'm trying to take things and it's why I've been 
shuffling things around and making things more generic.

> CPPLanguageRuntime is really only used in other plugins, so it's easy.  
> ObjCLanguageRuntime has a few more uses in generic code, and the 
> SwiftLanguageRuntime has some more uses in generic code (included in 16 
> generic files).  That corresponds to the fact that ObjC and the Swift are 
> systems that are increasingly more dependent on their runtimes in order for 
> the debugger to comprehend them.  Presumably we could fix this by adding some 
> more abstract methods to LanguageRuntime, though I'd have to do some more 
> investigation to see how awkward that would be, particularly for swift...

ObjCLanguageRuntime has 3 more uses in "base lldb" (non-plugin libraries) that 
I'm trying to get rid of.

- `Core/ValueObject.cpp`: D64159 
- `Expression/IRDynamicChecks.cpp`: D64591 
- `Symbol/ClangASTContext.cpp`: I don't plan on removing this one. Instead, I 
want to move ClangASTContext out of Symbol into a plugin.

After the first 2 are moved out, I intend on moving ObjCLanguageRuntime out as 
well. I'm also working on SwiftLanguageRuntime in the swift-lldb repository, 
although a little more slowly as I wanted to get C++ and ObjC done first.

Also, as an aside, I intend on trying to get swift debugging support into 
llvm.org at some point. I don't think llvm.org is in a place where that can be 
done cleanly, and I think swift-lldb's swift-specific bits need to be pulled 
into plugins before it becomes a reasonable endeavor. But that's the direction 
I want to head in. I feel similarly about Rust support. :)

> It doesn't seem to me useful to move CPPLanguageRuntime to the 
> instance-specific part of LanguageRuntime, if we can't also do that for ObjC 
> and Swift.  That's just confusing.  But if as a proof of concept you can get 
> ObjCLanguageRuntime.h out of generic code, then presumably we can also do 
> that for Swift, and the project seems worthwhile.

I uploaded this first to get some feedback and introduce the idea of moving 
CPPLanguageRuntime, ObjCLanguageRuntime, and SwiftLanguageRuntime. I figured 
I'd get any issues out of the way with the easiest one.




Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:55
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h"
 

jingham wrote:
> xiaobai wrote:
> > labath wrote:
> > > compnerd wrote:
> > > > xiaobai wrote:
> > > > > JDevlieghere wrote:
> > > > > > What's the benefit of making this a separate plugin, as compared to 
> > > > > > making it part of `Plugins/Language/CPlusPlus`? 
> > > > > I view LanguageRuntimes as distinct from Languages and thus I think 
> > > > > they should go into their own plugins. However, I'm not against 
> > > > > moving this to `Plugins/Language/CPlusPlus` if you think it would 
> > > > > make more sense to do so for another reason (e.g. less plugins 
> > > > > overall?)
> > > > We do need the abstraction since there are multiple C++ runtimes: c++, 
> > > > stdc++, MSVCPRT, stlport, etc.  Each one is slightly different.  
> > > > Furthermore, libstdc++ supported the GNU and Solaris ABIs, libc++ only 
> > > > does itanium, MSVCPRT only does MSVC ABI.  So, we need to have some 
> > > > layer to differentiate between the various ABIs and just general C++ 
> > > > language support.
> > > That is true. However, I'm not sure whether the current boundary actually 
> > > makes sense. E.g. the c++ language plugin implements pretty printers for 
> > > both libc++ and libstdc++.
> > Given that those are formatters specific to the libc++ and libstdc++ 
> > implementations, I think it would make sense that those are a part of the 
> > C++ language runtime plugin(s).
> So the problem with that is that formatters don't actually require a live 
> process to be useful.  They can be used to view static data in a not-running 
> process.  But LanguageRuntimes currently only come from a process.  So to 
> move the formatters to the LanguageRuntime - which does make some sense - 
> you're going to have to change the LanguageRuntime to do some things for a 
> target with no process and the more things when the Process gets a target.  
> That's not a bad change, BTW.
> 
> Anyway, that's why the formatters are a little out-of-place.
Right this makes sense to me. I think we could figure something out here, as 
I've dealt with needing access to 

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

This is a little unclear to me.  LLVMUserExpression is the class that 
represents "anything that uses LLVM at its back end."  Both the Swift & Clang 
user expressions are instances of this.  Running through IR and inserting 
little callouts is an LLVMUserExpression type thing.  So it seems like this 
move is in the right direction, but to be really clean needs to represent 
LLVMUserExpression in the Plugin Hierarchy.


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin

[Jim Ingham via Phabricator via lldb-commits]

jingham added inline comments.



Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:55
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h"
 

xiaobai wrote:
> labath wrote:
> > compnerd wrote:
> > > xiaobai wrote:
> > > > JDevlieghere wrote:
> > > > > What's the benefit of making this a separate plugin, as compared to 
> > > > > making it part of `Plugins/Language/CPlusPlus`? 
> > > > I view LanguageRuntimes as distinct from Languages and thus I think 
> > > > they should go into their own plugins. However, I'm not against moving 
> > > > this to `Plugins/Language/CPlusPlus` if you think it would make more 
> > > > sense to do so for another reason (e.g. less plugins overall?)
> > > We do need the abstraction since there are multiple C++ runtimes: c++, 
> > > stdc++, MSVCPRT, stlport, etc.  Each one is slightly different.  
> > > Furthermore, libstdc++ supported the GNU and Solaris ABIs, libc++ only 
> > > does itanium, MSVCPRT only does MSVC ABI.  So, we need to have some layer 
> > > to differentiate between the various ABIs and just general C++ language 
> > > support.
> > That is true. However, I'm not sure whether the current boundary actually 
> > makes sense. E.g. the c++ language plugin implements pretty printers for 
> > both libc++ and libstdc++.
> Given that those are formatters specific to the libc++ and libstdc++ 
> implementations, I think it would make sense that those are a part of the C++ 
> language runtime plugin(s).
So the problem with that is that formatters don't actually require a live 
process to be useful.  They can be used to view static data in a not-running 
process.  But LanguageRuntimes currently only come from a process.  So to move 
the formatters to the LanguageRuntime - which does make some sense - you're 
going to have to change the LanguageRuntime to do some things for a target with 
no process and the more things when the Process gets a target.  That's not a 
bad change, BTW.

Anyway, that's why the formatters are a little out-of-place.


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

https://reviews.llvm.org/D64599



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

[Lldb-commits] [PATCH] D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D64599#1581620 , @labath wrote:

> In D64599#1581598 , @jingham wrote:
>
> > My model for this was that there was a CPPLanguageRuntime.cpp that contains 
> > everything you can implement about the CPP runtime that is independent of 
> > any particular implementation of the CPP language runtime, and then a 
> > plugin instance (in this case the ItaniumABILanguageRuntime) that contains 
> > all the bits that are specific to a particular implementation.  Then 
> > putting the CPPLanguageRuntime.cpp in Target lets you know that this has to 
> > only contain the generic parts of the runtime behavior.  That still seems 
> > to me a useful distinction.
>
>
> I think that's fine, but it does not conflict the idea of the generic parts 
> of a cpp language runtime being a plugin also. This way the generic parts of 
> lldb would be truly language agnostic. Then if anything wants to work with a 
> c++ runtime, but it does not care which one, it can use CPPLanguageRuntime. 
> And if you need a very specific runtime, you go for the Itanium thingy.


I see where you are going...

Im my head, the plugin hierarchy for runtimes goes LanguageRuntime -> 
CPPLanguageRuntime -> ItaniumABILanguageRuntime.  I've been calling all those 
"parts of the plugin for language runtimes".   LanguageRuntime is the generic 
plugin interface, and currently CPPLanguageRuntime (and ObjCLanguageRuntime...) 
are also being treated as "generic parts of the LanguageRuntime plugin".  Then 
ItaniumABILanguageRuntime.h is an instance of the plugin.  Then the generic 
plugin interface files live out of the Plugin hierarchy and are free game, and 
the instances are in Plugins and should not be included from generic code.

Anyway, if you can make all the generic parts of lldb not depend on the 
language specific - but still abstract - part of the plugin, that would be 
fine.  Then just LanguageRuntime.h would live in Target, and CPPLanguageRuntime 
would live in Plugins/Language

CPPLanguageRuntime is really only used in other plugins, so it's easy.  
ObjCLanguageRuntime has a few more uses in generic code, and the 
SwiftLanguageRuntime has some more uses in generic code (included in 16 generic 
files).  That corresponds to the fact that ObjC and the Swift are systems that 
are increasingly more dependent on their runtimes in order for the debugger to 
comprehend them.  Presumably we could fix this by adding some more abstract 
methods to LanguageRuntime, though I'd have to do some more investigation to 
see how awkward that would be, particularly for swift...

It doesn't seem to me useful to move CPPLanguageRuntime to the 
instance-specific part of LanguageRuntime, if we can't also do that for ObjC 
and Swift.  That's just confusing.  But if as a proof of concept you can get 
ObjCLanguageRuntime.h out of generic code, then presumably we can also do that 
for Swift, and the project seems worthwhile.


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

https://reviews.llvm.org/D64599



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

[Lldb-commits] [PATCH] D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin

[Alex Langford via Phabricator via lldb-commits]

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

In D64599#1581620 , @labath wrote:

> In D64599#1581598 , @jingham wrote:
>
> > My model for this was that there was a CPPLanguageRuntime.cpp that contains 
> > everything you can implement about the CPP runtime that is independent of 
> > any particular implementation of the CPP language runtime, and then a 
> > plugin instance (in this case the ItaniumABILanguageRuntime) that contains 
> > all the bits that are specific to a particular implementation.  Then 
> > putting the CPPLanguageRuntime.cpp in Target lets you know that this has to 
> > only contain the generic parts of the runtime behavior.  That still seems 
> > to me a useful distinction.
>
>
> I think that's fine, but it does not conflict the idea of the generic parts 
> of a cpp language runtime being a plugin also. This way the generic parts of 
> lldb would be truly language agnostic. Then if anything wants to work with a 
> c++ runtime, but it does not care which one, it can use CPPLanguageRuntime. 
> And if you need a very specific runtime, you go for the Itanium thingy.


+1

In D64599#1581661 , @jingham wrote:

> In D64599#1581604 , @labath wrote:
>
> > What is the indented relationship between CPPLanguage and 
> > CPPLanguageRuntime plugins (and generally between any Language and its 
> > LanguageRuntime)? Right now you're having the CPPLanguage depend on the 
> > CPPLanguageRuntime plugin. There is no reverse dependency, so this may be 
> > fine, if that's how we intend things to be. However, it's not clear to me 
> > whether that's the right way to organize things. Intuitively, I'd expect 
> > the LanguageRuntime to depend on Language, and not the other way around...
>
>
> The nominal distinction is that the Language files contain what you can know 
> about or need to do with a given language without having a process and thus a 
> live runtime to query.  The LanguageRuntimes are supposed to be about what 
> you can gather from the actual runtime, or what you need to do to handle 
> things like "stepping into steps across ObjC method dispatch functions" or 
> "stepping into std::function steps in to the target function".  Given this 
> distinction you'd actually expect the two to be independent of one another, 
> but then it turns out that in both ObjC and Swift, you can't actually know 
> the sizes of objects, or the offsets of ivar members until you have a running 
> process.  So things you'd think the Language would know actually require the 
> LanguageRuntime...


I also think that Language depending on LanguageRuntime makes sense. It seems 
to me that in order for the Language to be able to answer questions about 
implementation-defined features, it must ask the language runtime for help.




Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:55
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h"
 

labath wrote:
> compnerd wrote:
> > xiaobai wrote:
> > > JDevlieghere wrote:
> > > > What's the benefit of making this a separate plugin, as compared to 
> > > > making it part of `Plugins/Language/CPlusPlus`? 
> > > I view LanguageRuntimes as distinct from Languages and thus I think they 
> > > should go into their own plugins. However, I'm not against moving this to 
> > > `Plugins/Language/CPlusPlus` if you think it would make more sense to do 
> > > so for another reason (e.g. less plugins overall?)
> > We do need the abstraction since there are multiple C++ runtimes: c++, 
> > stdc++, MSVCPRT, stlport, etc.  Each one is slightly different.  
> > Furthermore, libstdc++ supported the GNU and Solaris ABIs, libc++ only does 
> > itanium, MSVCPRT only does MSVC ABI.  So, we need to have some layer to 
> > differentiate between the various ABIs and just general C++ language 
> > support.
> That is true. However, I'm not sure whether the current boundary actually 
> makes sense. E.g. the c++ language plugin implements pretty printers for both 
> libc++ and libstdc++.
Given that those are formatters specific to the libc++ and libstdc++ 
implementations, I think it would make sense that those are a part of the C++ 
language runtime plugin(s).


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

https://reviews.llvm.org/D64599



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

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

Remove documentation boilerplate


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

https://reviews.llvm.org/D64591

Files:
  include/lldb/Expression/DynamicCheckerFunctions.h
  include/lldb/Expression/IRDynamicChecks.h
  source/Expression/CMakeLists.txt
  source/Expression/IRDynamicChecks.cpp
  source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
  source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
  source/Target/Process.cpp
  source/Target/ThreadPlanCallUserExpression.cpp

Index: source/Target/ThreadPlanCallUserExpression.cpp
===
--- source/Target/ThreadPlanCallUserExpression.cpp
+++ source/Target/ThreadPlanCallUserExpression.cpp
@@ -13,7 +13,7 @@
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Target/LanguageRuntime.h"
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -22,7 +22,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
Index: source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
===
--- source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
+++ source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
@@ -9,46 +9,32 @@
 #ifndef liblldb_IRDynamicChecks_h_
 #define liblldb_IRDynamicChecks_h_
 
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/lldb-types.h"
 #include "llvm/Pass.h"
 
 namespace llvm {
 class BasicBlock;
-class CallInst;
-class Constant;
-class Function;
-class Instruction;
 class Module;
-class DataLayout;
-class Value;
 }
 
 namespace lldb_private {
 
-class ClangExpressionDeclMap;
 class ExecutionContext;
 class Stream;
 
-/// \class DynamicCheckerFunctions IRDynamicChecks.h
-/// "lldb/Expression/IRDynamicChecks.h" Encapsulates dynamic check functions
-/// used by expressions.
-///
-/// Each of the utility functions encapsulated in this class is responsible
-/// for validating some data that an expression is about to use.  Examples
-/// are:
-///
-/// a = *b; // check that b is a valid pointer [b init];   // check that b
-/// is a valid object to send "init" to
-///
-/// The class installs each checker function into the target process and makes
-/// it available to IRDynamicChecks to use.
-class DynamicCheckerFunctions {
+class ClangDynamicCheckerFunctions
+: public lldb_private::DynamicCheckerFunctions {
 public:
   /// Constructor
-  DynamicCheckerFunctions();
+  ClangDynamicCheckerFunctions();
 
   /// Destructor
-  ~DynamicCheckerFunctions();
+  virtual ~ClangDynamicCheckerFunctions();
+
+  static bool classof(const DynamicCheckerFunctions *checker_funcs) {
+return checker_funcs->GetKind() == DCF_Clang;
+  }
 
   /// Install the utility functions into a process.  This binds the instance
   /// of DynamicCheckerFunctions to that process.
@@ -63,9 +49,9 @@
   /// True on success; false on failure, or if the functions have
   /// already been installed.
   bool Install(DiagnosticManager _manager,
-   ExecutionContext _ctx);
+   ExecutionContext _ctx) override;
 
-  bool DoCheckersExplainStop(lldb::addr_t addr, Stream );
+  bool DoCheckersExplainStop(lldb::addr_t addr, Stream ) override;
 
   std::shared_ptr m_valid_pointer_check;
   std::shared_ptr m_objc_object_check;
@@ -94,7 +80,7 @@
   /// \param[in] decl_map
   /// The mapping used to look up entities in the target process. In
   /// this case, used to find objc_msgSend
-  IRDynamicChecks(DynamicCheckerFunctions _functions,
+  IRDynamicChecks(ClangDynamicCheckerFunctions _functions,
   const char *func_name = "$__lldb_expr");
 
   /// Destructor
@@ -136,7 +122,7 @@
   bool FindDataLoads(llvm::Module , llvm::BasicBlock );
 
   std::string m_func_name; ///< The name of the function to add checks to
-  DynamicCheckerFunctions
+  ClangDynamicCheckerFunctions
   _checker_functions; ///< The checker functions for the process
 };
 
Index: source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
===
--- 

[Lldb-commits] [PATCH] D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D64599#1581604 , @labath wrote:

> What is the indented relationship between CPPLanguage and CPPLanguageRuntime 
> plugins (and generally between any Language and its LanguageRuntime)? Right 
> now you're having the CPPLanguage depend on the CPPLanguageRuntime plugin. 
> There is no reverse dependency, so this may be fine, if that's how we intend 
> things to be. However, it's not clear to me whether that's the right way to 
> organize things. Intuitively, I'd expect the LanguageRuntime to depend on 
> Language, and not the other way around...


The nominal distinction is that the Language files contain what you can know 
about or need to do with a given language without having a process and thus a 
live runtime to query.  The LanguageRuntimes are supposed to be about what you 
can gather from the actual runtime, or what you need to do to handle things 
like "stepping into steps across ObjC method dispatch functions" or "stepping 
into std::function steps in to the target function".  Given this distinction 
you'd actually expect the two to be independent of one another, but then it 
turns out that in both ObjC and Swift, you can't actually know the sizes of 
objects, or the offsets of ivar members until you have a running process.  So 
things you'd think the Language would know actually require the 
LanguageRuntime...


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

https://reviews.llvm.org/D64599



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

[Lldb-commits] [PATCH] D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:55
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h"
 

compnerd wrote:
> xiaobai wrote:
> > JDevlieghere wrote:
> > > What's the benefit of making this a separate plugin, as compared to 
> > > making it part of `Plugins/Language/CPlusPlus`? 
> > I view LanguageRuntimes as distinct from Languages and thus I think they 
> > should go into their own plugins. However, I'm not against moving this to 
> > `Plugins/Language/CPlusPlus` if you think it would make more sense to do so 
> > for another reason (e.g. less plugins overall?)
> We do need the abstraction since there are multiple C++ runtimes: c++, 
> stdc++, MSVCPRT, stlport, etc.  Each one is slightly different.  Furthermore, 
> libstdc++ supported the GNU and Solaris ABIs, libc++ only does itanium, 
> MSVCPRT only does MSVC ABI.  So, we need to have some layer to differentiate 
> between the various ABIs and just general C++ language support.
That is true. However, I'm not sure whether the current boundary actually makes 
sense. E.g. the c++ language plugin implements pretty printers for both libc++ 
and libstdc++.


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

https://reviews.llvm.org/D64599



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

[Lldb-commits] [PATCH] D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D64599#1581598 , @jingham wrote:

> My model for this was that there was a CPPLanguageRuntime.cpp that contains 
> everything you can implement about the CPP runtime that is independent of any 
> particular implementation of the CPP language runtime, and then a plugin 
> instance (in this case the ItaniumABILanguageRuntime) that contains all the 
> bits that are specific to a particular implementation.  Then putting the 
> CPPLanguageRuntime.cpp in Target lets you know that this has to only contain 
> the generic parts of the runtime behavior.  That still seems to me a useful 
> distinction.


I think that's fine, but it does not conflict the idea of the generic parts of 
a cpp language runtime being a plugin also. This way the generic parts of lldb 
would be truly language agnostic. Then if anything wants to work with a c++ 
runtime, but it does not care which one, it can use CPPLanguageRuntime. And if 
you need a very specific runtime, you go for the Itanium thingy.


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

https://reviews.llvm.org/D64599



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

[Lldb-commits] [PATCH] D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added inline comments.



Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:55
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h"
 

xiaobai wrote:
> JDevlieghere wrote:
> > What's the benefit of making this a separate plugin, as compared to making 
> > it part of `Plugins/Language/CPlusPlus`? 
> I view LanguageRuntimes as distinct from Languages and thus I think they 
> should go into their own plugins. However, I'm not against moving this to 
> `Plugins/Language/CPlusPlus` if you think it would make more sense to do so 
> for another reason (e.g. less plugins overall?)
We do need the abstraction since there are multiple C++ runtimes: c++, stdc++, 
MSVCPRT, stlport, etc.  Each one is slightly different.  Furthermore, libstdc++ 
supported the GNU and Solaris ABIs, libc++ only does itanium, MSVCPRT only does 
MSVC ABI.  So, we need to have some layer to differentiate between the various 
ABIs and just general C++ language support.


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

https://reviews.llvm.org/D64599



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: include/lldb/Expression/DynamicCheckerFunctions.h:19
+
+/// \class DynamicCheckerFunctions DynamicCheckerFunctions.h
+/// "lldb/Expression/DynamicCheckerFunctions.h" Encapsulates dynamic check

xiaobai wrote:
> JDevlieghere wrote:
> > Can we skip this boilerplate? 
> Do you mean the whole thing? Or just the `\class` line?
Everything up to "Encapsulates dynamic check...", Doxygen is smart enough to 
associate the comment with the class. 


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin

[Alex Langford via Phabricator via lldb-commits]

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



Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:55
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h"
 

JDevlieghere wrote:
> What's the benefit of making this a separate plugin, as compared to making it 
> part of `Plugins/Language/CPlusPlus`? 
I view LanguageRuntimes as distinct from Languages and thus I think they should 
go into their own plugins. However, I'm not against moving this to 
`Plugins/Language/CPlusPlus` if you think it would make more sense to do so for 
another reason (e.g. less plugins overall?)


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

https://reviews.llvm.org/D64599



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

[Lldb-commits] [PATCH] D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

What is the indented relationship between CPPLanguage and CPPLanguageRuntime 
plugins (and generally between any Language and its LanguageRuntime)? Right now 
you're having the CPPLanguage depend on the CPPLanguageRuntime plugin. There is 
no reverse dependency, so this may be fine, if that's how we intend things to 
be. However, it's not clear to me whether that's the right way to organize 
things. Intuitively, I'd expect the LanguageRuntime to depend on Language, and 
not the other way around...


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

https://reviews.llvm.org/D64599



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

[Lldb-commits] [PATCH] D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin

[Jim Ingham via Phabricator via lldb-commits]

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

My model for this was that there was a CPPLanguageRuntime.cpp that contains 
everything you can implement about the CPP runtime that is independent of any 
particular implementation of the CPP language runtime, and then a plugin 
instance (in this case the ItaniumABILanguageRuntime) that contains all the 
bits that are specific to a particular implementation.  Then putting the 
CPPLanguageRuntime.cpp in Target lets you know that this has to only contain 
the generic parts of the runtime behavior.  That still seems to me a useful 
distinction.


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

https://reviews.llvm.org/D64599



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

xiaobai added inline comments.



Comment at: include/lldb/Expression/DynamicCheckerFunctions.h:19
+
+/// \class DynamicCheckerFunctions DynamicCheckerFunctions.h
+/// "lldb/Expression/DynamicCheckerFunctions.h" Encapsulates dynamic check

JDevlieghere wrote:
> Can we skip this boilerplate? 
Do you mean the whole thing? Or just the `\class` line?


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:55
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h"
 

What's the benefit of making this a separate plugin, as compared to making it 
part of `Plugins/Language/CPlusPlus`? 


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

https://reviews.llvm.org/D64599



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

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

Reflow comment
Remove newline


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

https://reviews.llvm.org/D64591

Files:
  include/lldb/Expression/DynamicCheckerFunctions.h
  include/lldb/Expression/IRDynamicChecks.h
  source/Expression/CMakeLists.txt
  source/Expression/IRDynamicChecks.cpp
  source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
  source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
  source/Target/Process.cpp
  source/Target/ThreadPlanCallUserExpression.cpp

Index: source/Target/ThreadPlanCallUserExpression.cpp
===
--- source/Target/ThreadPlanCallUserExpression.cpp
+++ source/Target/ThreadPlanCallUserExpression.cpp
@@ -13,7 +13,7 @@
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Target/LanguageRuntime.h"
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -22,7 +22,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
Index: source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
===
--- source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
+++ source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
@@ -9,46 +9,32 @@
 #ifndef liblldb_IRDynamicChecks_h_
 #define liblldb_IRDynamicChecks_h_
 
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/lldb-types.h"
 #include "llvm/Pass.h"
 
 namespace llvm {
 class BasicBlock;
-class CallInst;
-class Constant;
-class Function;
-class Instruction;
 class Module;
-class DataLayout;
-class Value;
 }
 
 namespace lldb_private {
 
-class ClangExpressionDeclMap;
 class ExecutionContext;
 class Stream;
 
-/// \class DynamicCheckerFunctions IRDynamicChecks.h
-/// "lldb/Expression/IRDynamicChecks.h" Encapsulates dynamic check functions
-/// used by expressions.
-///
-/// Each of the utility functions encapsulated in this class is responsible
-/// for validating some data that an expression is about to use.  Examples
-/// are:
-///
-/// a = *b; // check that b is a valid pointer [b init];   // check that b
-/// is a valid object to send "init" to
-///
-/// The class installs each checker function into the target process and makes
-/// it available to IRDynamicChecks to use.
-class DynamicCheckerFunctions {
+class ClangDynamicCheckerFunctions
+: public lldb_private::DynamicCheckerFunctions {
 public:
   /// Constructor
-  DynamicCheckerFunctions();
+  ClangDynamicCheckerFunctions();
 
   /// Destructor
-  ~DynamicCheckerFunctions();
+  virtual ~ClangDynamicCheckerFunctions();
+
+  static bool classof(const DynamicCheckerFunctions *checker_funcs) {
+return checker_funcs->GetKind() == DCF_Clang;
+  }
 
   /// Install the utility functions into a process.  This binds the instance
   /// of DynamicCheckerFunctions to that process.
@@ -63,9 +49,9 @@
   /// True on success; false on failure, or if the functions have
   /// already been installed.
   bool Install(DiagnosticManager _manager,
-   ExecutionContext _ctx);
+   ExecutionContext _ctx) override;
 
-  bool DoCheckersExplainStop(lldb::addr_t addr, Stream );
+  bool DoCheckersExplainStop(lldb::addr_t addr, Stream ) override;
 
   std::shared_ptr m_valid_pointer_check;
   std::shared_ptr m_objc_object_check;
@@ -94,7 +80,7 @@
   /// \param[in] decl_map
   /// The mapping used to look up entities in the target process. In
   /// this case, used to find objc_msgSend
-  IRDynamicChecks(DynamicCheckerFunctions _functions,
+  IRDynamicChecks(ClangDynamicCheckerFunctions _functions,
   const char *func_name = "$__lldb_expr");
 
   /// Destructor
@@ -136,7 +122,7 @@
   bool FindDataLoads(llvm::Module , llvm::BasicBlock );
 
   std::string m_func_name; ///< The name of the function to add checks to
-  DynamicCheckerFunctions
+  ClangDynamicCheckerFunctions
   _checker_functions; ///< The checker functions for the process
 };
 
Index: source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
===
--- 

[Lldb-commits] [PATCH] D64599: [LanguageRuntime] Move CPPLanguageRuntime into a plugin

[Alex Langford via Phabricator via lldb-commits]

xiaobai created this revision.
xiaobai added reviewers: JDevlieghere, clayborg, jingham, compnerd, labath.
Herald added a subscriber: mgorny.

This seems better suited to be in a plugin.


https://reviews.llvm.org/D64599

Files:
  include/lldb/Target/CPPLanguageRuntime.h
  source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/LanguageRuntime/CPlusPlus/CMakeLists.txt
  source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
  source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
  source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/CMakeLists.txt
  
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.h
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/CMakeLists.txt
  
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
  source/Target/CMakeLists.txt
  source/Target/CPPLanguageRuntime.cpp

Index: source/Target/CMakeLists.txt
===
--- source/Target/CMakeLists.txt
+++ source/Target/CMakeLists.txt
@@ -1,6 +1,5 @@
 add_lldb_library(lldbTarget
   ABI.cpp
-  CPPLanguageRuntime.cpp
   ExecutionContext.cpp
   JITLoader.cpp
   JITLoaderList.cpp
Index: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
===
--- source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
+++ source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
@@ -19,10 +19,11 @@
 #include "llvm/ADT/StringRef.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Expression/LLVMUserExpression.h"
-#include "lldb/Target/CPPLanguageRuntime.h"
 #include "lldb/Target/LanguageRuntime.h"
 #include "lldb/lldb-private.h"
 
+#include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h"
+
 namespace lldb_private {
 namespace lldb_renderscript {
 
Index: source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/CMakeLists.txt
===
--- source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/CMakeLists.txt
+++ source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/CMakeLists.txt
@@ -19,6 +19,7 @@
 lldbTarget
 lldbUtility
 lldbPluginExpressionParserClang
+lldbPluginCPPRuntime
   LINK_COMPONENTS
 Support
   )
Index: source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
===
--- source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -24,7 +24,6 @@
 #include "lldb/Expression/FunctionCaller.h"
 #include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Symbol/ObjectFile.h"
-#include "lldb/Target/CPPLanguageRuntime.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
@@ -39,6 +38,7 @@
 
 #include "Plugins/Process/Utility/HistoryThread.h"
 #include "Plugins/Language/ObjC/NSString.h"
+#include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h"
 
 #include 
 
Index: source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.h
===
--- source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.h
+++ source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.h
@@ -16,10 +16,11 @@
 #include "lldb/Breakpoint/BreakpointResolver.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Symbol/Type.h"
-#include "lldb/Target/CPPLanguageRuntime.h"
 #include "lldb/Target/LanguageRuntime.h"
 #include "lldb/lldb-private.h"
 
+#include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h"
+
 namespace lldb_private {
 
 class ItaniumABILanguageRuntime : public lldb_private::CPPLanguageRuntime {
Index: source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/CMakeLists.txt
===
--- source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/CMakeLists.txt
+++ source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/CMakeLists.txt
@@ -7,4 +7,5 @@
 lldbInterpreter
 lldbSymbol
 lldbTarget
+lldbPluginCPPRuntime
   )
Index: include/lldb/Target/CPPLanguageRuntime.h
===
--- /dev/null
+++ include/lldb/Target/CPPLanguageRuntime.h
@@ -1,90 +0,0 @@
-//===-- CPPLanguageRuntime.h
-//
-// 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
-//

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: include/lldb/Expression/DynamicCheckerFunctions.h:19
+
+/// \class DynamicCheckerFunctions DynamicCheckerFunctions.h
+/// "lldb/Expression/DynamicCheckerFunctions.h" Encapsulates dynamic check

Can we skip this boilerplate? 



Comment at: include/lldb/Expression/DynamicCheckerFunctions.h:27
+///
+/// a = *b; // check that b is a valid pointer [b init];   // check that b
+/// is a valid object to send "init" to

Reflow comment.



Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:1307
+process->GetDynamicCheckers())) {
 
+  IRDynamicChecks ir_dynamic_checks(*checker_funcs,

Spurious newline?


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64583: [lldb][test_suite] Fix skipIfTargetAndroid decorator

[Wanyi Ye via Phabricator via lldb-commits]

kusmour updated this revision to Diff 209325.
kusmour edited the summary of this revision.
kusmour added a comment.
Herald added a subscriber: abidh.

add more context to diff


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D64583

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py


Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -596,18 +596,13 @@
 """Decorate the item to skip tests that should be skipped on Windows."""
 return skipIfPlatform(["windows"])(func)

-def skipIfTargetAndroid(func):
-return unittest2.skipIf(lldbplatformutil.target_is_android(),
-"skip on target Android")(func)
-
-
 def skipUnlessWindows(func):
 """Decorate the item to skip tests that should be skipped on any 
non-Windows platform."""
 return skipUnlessPlatform(["windows"])(func)


 def skipUnlessDarwin(func):
 """Decorate the item to skip tests that should be skipped on any non 
Darwin platform."""
 return skipUnlessPlatform(lldbplatformutil.getDarwinOSTriples())(func)

 def skipUnlessTargetAndroid(func):
@@ -652,9 +647,9 @@
 "requires one of %s" % (", ".join(oslist)))


-def skipIfTargetAndroid(api_levels=None, archs=None):
+def skipIfTargetAndroid(bugnumber=None, api_levels=None, archs=None):
 """Decorator to skip tests when the target is Android.

 Arguments:
 api_levels - The API levels for which the test should be skipped. If
 it is None, then the test will be skipped for all API levels.
@@ -665,8 +660,9 @@
 _skip_for_android(
 "skipping for android",
 api_levels,
-archs))
+archs),
+bugnumber)

 def skipUnlessSupportedTypeAttribute(attr):
 """Decorate the item to skip test unless Clang supports type 
__attribute__(attr)."""
 def compiler_doesnt_support_struct_attribute(self):


Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -596,18 +596,13 @@
 """Decorate the item to skip tests that should be skipped on Windows."""
 return skipIfPlatform(["windows"])(func)

-def skipIfTargetAndroid(func):
-return unittest2.skipIf(lldbplatformutil.target_is_android(),
-"skip on target Android")(func)
-
-
 def skipUnlessWindows(func):
 """Decorate the item to skip tests that should be skipped on any non-Windows platform."""
 return skipUnlessPlatform(["windows"])(func)


 def skipUnlessDarwin(func):
 """Decorate the item to skip tests that should be skipped on any non Darwin platform."""
 return skipUnlessPlatform(lldbplatformutil.getDarwinOSTriples())(func)

 def skipUnlessTargetAndroid(func):
@@ -652,9 +647,9 @@
 "requires one of %s" % (", ".join(oslist)))


-def skipIfTargetAndroid(api_levels=None, archs=None):
+def skipIfTargetAndroid(bugnumber=None, api_levels=None, archs=None):
 """Decorator to skip tests when the target is Android.

 Arguments:
 api_levels - The API levels for which the test should be skipped. If
 it is None, then the test will be skipped for all API levels.
@@ -665,8 +660,9 @@
 _skip_for_android(
 "skipping for android",
 api_levels,
-archs))
+archs),
+bugnumber)

 def skipUnlessSupportedTypeAttribute(attr):
 """Decorate the item to skip test unless Clang supports type __attribute__(attr)."""
 def compiler_doesnt_support_struct_attribute(self):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D64583: [lldb][test_suite] Fix skipIfTargetAndroid decorator

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

Can you add more context to this diff? I can't see the code surrounding the 
changes. You can get the whole context with something like `git show -U`.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D64583



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

xiaobai created this revision.
xiaobai added reviewers: compnerd, clayborg, jingham, JDevlieghere.
Herald added a subscriber: mgorny.

IRDynamicChecks in its current form is specific to Clang since it deals
with the C language family. It is possible that we may want to
instrument code generated for other languages, but we can factor in a
more general mechanism to do so at a later time.

This decouples ObCLanguageRuntime from Expression!


https://reviews.llvm.org/D64591

Files:
  include/lldb/Expression/DynamicCheckerFunctions.h
  include/lldb/Expression/IRDynamicChecks.h
  source/Expression/CMakeLists.txt
  source/Expression/IRDynamicChecks.cpp
  source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
  source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
  source/Target/Process.cpp
  source/Target/ThreadPlanCallUserExpression.cpp

Index: source/Target/ThreadPlanCallUserExpression.cpp
===
--- source/Target/ThreadPlanCallUserExpression.cpp
+++ source/Target/ThreadPlanCallUserExpression.cpp
@@ -13,7 +13,7 @@
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Target/LanguageRuntime.h"
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -22,7 +22,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
Index: source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
===
--- source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
+++ source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
@@ -9,46 +9,32 @@
 #ifndef liblldb_IRDynamicChecks_h_
 #define liblldb_IRDynamicChecks_h_
 
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/lldb-types.h"
 #include "llvm/Pass.h"
 
 namespace llvm {
 class BasicBlock;
-class CallInst;
-class Constant;
-class Function;
-class Instruction;
 class Module;
-class DataLayout;
-class Value;
 }
 
 namespace lldb_private {
 
-class ClangExpressionDeclMap;
 class ExecutionContext;
 class Stream;
 
-/// \class DynamicCheckerFunctions IRDynamicChecks.h
-/// "lldb/Expression/IRDynamicChecks.h" Encapsulates dynamic check functions
-/// used by expressions.
-///
-/// Each of the utility functions encapsulated in this class is responsible
-/// for validating some data that an expression is about to use.  Examples
-/// are:
-///
-/// a = *b; // check that b is a valid pointer [b init];   // check that b
-/// is a valid object to send "init" to
-///
-/// The class installs each checker function into the target process and makes
-/// it available to IRDynamicChecks to use.
-class DynamicCheckerFunctions {
+class ClangDynamicCheckerFunctions
+: public lldb_private::DynamicCheckerFunctions {
 public:
   /// Constructor
-  DynamicCheckerFunctions();
+  ClangDynamicCheckerFunctions();
 
   /// Destructor
-  ~DynamicCheckerFunctions();
+  virtual ~ClangDynamicCheckerFunctions();
+
+  static bool classof(const DynamicCheckerFunctions *checker_funcs) {
+return checker_funcs->GetKind() == DCF_Clang;
+  }
 
   /// Install the utility functions into a process.  This binds the instance
   /// of DynamicCheckerFunctions to that process.
@@ -63,9 +49,9 @@
   /// True on success; false on failure, or if the functions have
   /// already been installed.
   bool Install(DiagnosticManager _manager,
-   ExecutionContext _ctx);
+   ExecutionContext _ctx) override;
 
-  bool DoCheckersExplainStop(lldb::addr_t addr, Stream );
+  bool DoCheckersExplainStop(lldb::addr_t addr, Stream ) override;
 
   std::shared_ptr m_valid_pointer_check;
   std::shared_ptr m_objc_object_check;
@@ -94,7 +80,7 @@
   /// \param[in] decl_map
   /// The mapping used to look up entities in the target process. In
   /// this case, used to find objc_msgSend
-  IRDynamicChecks(DynamicCheckerFunctions _functions,
+  IRDynamicChecks(ClangDynamicCheckerFunctions _functions,
   const char *func_name = "$__lldb_expr");
 
   /// Destructor
@@ -136,7 +122,7 @@
   bool FindDataLoads(llvm::Module , llvm::BasicBlock );
 
   std::string m_func_name; ///< The name of the function to add checks to
-  DynamicCheckerFunctions
+  

[Lldb-commits] [PATCH] D64535: Add convenience methods to convert LLDB to LLVM data structures.

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365819: Add convenience methods to convert LLDB to LLVM data 
structures. (authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64535?vs=209107=209313#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64535

Files:
  lldb/trunk/include/lldb/Core/Section.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/trunk/tools/lldb-test/lldb-test.cpp

Index: lldb/trunk/include/lldb/Core/Section.h
===
--- lldb/trunk/include/lldb/Core/Section.h
+++ lldb/trunk/include/lldb/Core/Section.h
@@ -38,6 +38,11 @@
   typedef collection::iterator iterator;
   typedef collection::const_iterator const_iterator;
 
+  const_iterator begin() const { return m_sections.begin(); }
+  const_iterator end() const { return m_sections.end(); }
+  const_iterator begin() { return m_sections.begin(); }
+  const_iterator end() { return m_sections.end(); }
+
   SectionList();
 
   ~SectionList();
Index: lldb/trunk/tools/lldb-test/lldb-test.cpp
===
--- lldb/trunk/tools/lldb-test/lldb-test.cpp
+++ lldb/trunk/tools/lldb-test/lldb-test.cpp
@@ -739,7 +739,7 @@
 Printer.formatLine("File size: {0}", S->GetFileSize());
 
 if (opts::object::SectionContents) {
-  DataExtractor Data;
+  lldb_private::DataExtractor Data;
   S->GetSectionData(Data);
   ArrayRef Bytes = {Data.GetDataStart(), Data.GetDataEnd()};
   Printer.formatBinary("Data: ", Bytes, 0);
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -16,13 +16,6 @@
 using namespace lldb_private;
 using namespace lldb;
 
-static llvm::DWARFDataExtractor ToLLVM(const DWARFDataExtractor ) {
-  return llvm::DWARFDataExtractor(
-  llvm::StringRef(reinterpret_cast(data.GetDataStart()),
-  data.GetByteSize()),
-  data.GetByteOrder() == eByteOrderLittle, data.GetAddressByteSize());
-}
-
 llvm::Expected>
 DebugNamesDWARFIndex::Create(Module , DWARFDataExtractor debug_names,
  DWARFDataExtractor debug_str,
@@ -31,8 +24,8 @@
 return llvm::make_error("debug info null",
llvm::inconvertibleErrorCode());
   }
-  auto index_up =
-  llvm::make_unique(ToLLVM(debug_names), ToLLVM(debug_str));
+  auto index_up = llvm::make_unique(debug_names.GetAsLLVM(),
+debug_str.GetAsLLVM());
   if (llvm::Error E = index_up->extract())
 return std::move(E);
 
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "DWARFDataExtractor.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace lldb_private {
 
@@ -19,4 +20,11 @@
 DWARFDataExtractor::GetDWARFOffset(lldb::offset_t *offset_ptr) const {
   return GetMaxU64(offset_ptr, GetDWARFSizeOfOffset());
 }
+
+llvm::DWARFDataExtractor DWARFDataExtractor::GetAsLLVM() const {
+  return llvm::DWARFDataExtractor(
+  llvm::StringRef(reinterpret_cast(GetDataStart()),
+  GetByteSize()),
+  GetByteOrder() == lldb::eByteOrderLittle, GetAddressByteSize());
 }
+} // namespace lldb_private
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h
@@ -12,6 +12,7 @@
 #include "DWARFDataExtractor.h"
 #include "lldb/Core/Section.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/Support/Threading.h"
 #include 
 
@@ -20,6 +21,7 @@
 private:
   SectionList *m_main_section_list;
   SectionList *m_dwo_section_list;
+  mutable std::unique_ptr m_llvm_context;
 
   struct SectionData {
 llvm::once_flag flag;
@@ -64,6 +66,8 @@
   const DWARFDataExtractor ();
   const DWARFDataExtractor ();
   const DWARFDataExtractor ();

[Lldb-commits] [lldb] r365819 - Add convenience methods to convert LLDB to LLVM data structures.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Thu Jul 11 13:26:53 2019
New Revision: 365819

URL: http://llvm.org/viewvc/llvm-project?rev=365819=rev
Log:
Add convenience methods to convert LLDB to LLVM data structures.

This patch adds two convenience methods named GetAsLLVM to the LLDB
counterparts of the DWARF DataExtractor and the DWARF context. The
DWARFContext, once created, is cached for future usage.

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

Modified:
lldb/trunk/include/lldb/Core/Section.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
lldb/trunk/tools/lldb-test/lldb-test.cpp

Modified: lldb/trunk/include/lldb/Core/Section.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Section.h?rev=365819=365818=365819=diff
==
--- lldb/trunk/include/lldb/Core/Section.h (original)
+++ lldb/trunk/include/lldb/Core/Section.h Thu Jul 11 13:26:53 2019
@@ -38,6 +38,11 @@ public:
   typedef collection::iterator iterator;
   typedef collection::const_iterator const_iterator;
 
+  const_iterator begin() const { return m_sections.begin(); }
+  const_iterator end() const { return m_sections.end(); }
+  const_iterator begin() { return m_sections.begin(); }
+  const_iterator end() { return m_sections.end(); }
+
   SectionList();
 
   ~SectionList();

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp?rev=365819=365818=365819=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp Thu Jul 11 
13:26:53 2019
@@ -100,3 +100,37 @@ const DWARFDataExtractor ::
   return LoadOrGetSection(eSectionTypeDWARFDebugTypes,
   eSectionTypeDWARFDebugTypesDwo, m_data_debug_types);
 }
+
+llvm::DWARFContext ::GetAsLLVM() {
+  if (!m_llvm_context) {
+llvm::StringMap> section_map;
+uint8_t addr_size = 0;
+
+auto AddSection = [&](Section ) {
+  DataExtractor section_data;
+  section.GetSectionData(section_data);
+
+  // Set the address size the first time we see it.
+  if (addr_size == 0)
+addr_size = section_data.GetByteSize();
+
+  llvm::StringRef data = llvm::toStringRef(section_data.GetData());
+  llvm::StringRef name = section.GetName().GetStringRef();
+  section_map.try_emplace(
+  name, llvm::MemoryBuffer::getMemBuffer(data, name, false));
+};
+
+if (m_main_section_list) {
+  for (auto  : *m_main_section_list)
+AddSection(*section);
+}
+
+if (m_dwo_section_list) {
+  for (auto  : *m_dwo_section_list)
+AddSection(*section);
+}
+
+m_llvm_context = llvm::DWARFContext::create(section_map, addr_size);
+  }
+  return *m_llvm_context;
+}

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h?rev=365819=365818=365819=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h Thu Jul 11 
13:26:53 2019
@@ -12,6 +12,7 @@
 #include "DWARFDataExtractor.h"
 #include "lldb/Core/Section.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/Support/Threading.h"
 #include 
 
@@ -20,6 +21,7 @@ class DWARFContext {
 private:
   SectionList *m_main_section_list;
   SectionList *m_dwo_section_list;
+  mutable std::unique_ptr m_llvm_context;
 
   struct SectionData {
 llvm::once_flag flag;
@@ -64,6 +66,8 @@ public:
   const DWARFDataExtractor ();
   const DWARFDataExtractor ();
   const DWARFDataExtractor ();
+
+  llvm::DWARFContext ();
 };
 } // namespace lldb_private
 

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.cpp?rev=365819=365818=365819=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.cpp Thu Jul 
11 13:26:53 2019
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "DWARFDataExtractor.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace lldb_private {
 
@@ -19,4 +20,11 @@ 

[Lldb-commits] [PATCH] D64583: [lldb][test_suite] Fix skipIfTargetAndroid decorator

[Wanyi Ye via Phabricator via lldb-commits]

kusmour created this revision.
kusmour added a reviewer: xiaobai.
Herald added subscribers: lldb-commits, srhines.
Herald added a project: LLDB.

Delete the duplicate func `skipIfTargetAndroid`
Fix the old one


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D64583

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py


Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -596,11 +596,6 @@
 """Decorate the item to skip tests that should be skipped on Windows."""
 return skipIfPlatform(["windows"])(func)

-def skipIfTargetAndroid(func):
-return unittest2.skipIf(lldbplatformutil.target_is_android(),
-"skip on target Android")(func)
-
-
 def skipUnlessWindows(func):
 """Decorate the item to skip tests that should be skipped on any 
non-Windows platform."""
 return skipUnlessPlatform(["windows"])(func)
@@ -652,7 +647,7 @@
 "requires one of %s" % (", ".join(oslist)))


-def skipIfTargetAndroid(api_levels=None, archs=None):
+def skipIfTargetAndroid(bugnumber=None, api_levels=None, archs=None):
 """Decorator to skip tests when the target is Android.

 Arguments:
@@ -665,7 +660,8 @@
 _skip_for_android(
 "skipping for android",
 api_levels,
-archs))
+archs),
+bugnumber)

 def skipUnlessSupportedTypeAttribute(attr):
 """Decorate the item to skip test unless Clang supports type 
__attribute__(attr)."""


Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -596,11 +596,6 @@
 """Decorate the item to skip tests that should be skipped on Windows."""
 return skipIfPlatform(["windows"])(func)

-def skipIfTargetAndroid(func):
-return unittest2.skipIf(lldbplatformutil.target_is_android(),
-"skip on target Android")(func)
-
-
 def skipUnlessWindows(func):
 """Decorate the item to skip tests that should be skipped on any non-Windows platform."""
 return skipUnlessPlatform(["windows"])(func)
@@ -652,7 +647,7 @@
 "requires one of %s" % (", ".join(oslist)))


-def skipIfTargetAndroid(api_levels=None, archs=None):
+def skipIfTargetAndroid(bugnumber=None, api_levels=None, archs=None):
 """Decorator to skip tests when the target is Android.

 Arguments:
@@ -665,7 +660,8 @@
 _skip_for_android(
 "skipping for android",
 api_levels,
-archs))
+archs),
+bugnumber)

 def skipUnlessSupportedTypeAttribute(attr):
 """Decorate the item to skip test unless Clang supports type __attribute__(attr)."""
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D64546: [lldb] Make TestDeletedExecutable more reliable

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365813: [lldb] Make TestDeletedExecutable more reliable 
(authored by teemperor, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64546?vs=209231=209300#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64546

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp


Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp
@@ -1,9 +1,15 @@
 #include 
+#include 
 #include 
 
-int main(int argc, char const *argv[])
-{
+int main(int argc, char const *argv[]) {
 lldb_enable_attach();
 
-std::this_thread::sleep_for(std::chrono::seconds(30));
+{
+  // Create file to signal that this process has started up.
+  std::ofstream f;
+  f.open(argv[1]);
+}
+
+std::this_thread::sleep_for(std::chrono::seconds(60));
 }
Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
@@ -1,5 +1,5 @@
 """
-Test process attach/resume.
+Test process attach when executable was deleted.
 """
 
 from __future__ import print_function
@@ -24,8 +24,22 @@
 self.build()
 exe = self.getBuildArtifact("a.out")
 
-popen = self.spawnSubprocess(exe)
+# Use a file as a synchronization point between test and inferior.
+pid_file_path = lldbutil.append_to_process_working_directory(self,
+"token_pid_%d" % (int(os.getpid(
+self.addTearDownHook(
+lambda: self.run_platform_command(
+"rm %s" %
+(pid_file_path)))
+
+# Spawn a new process
+popen = self.spawnSubprocess(exe, [pid_file_path])
 self.addTearDownHook(self.cleanupSubprocesses)
+
+# Wait until process has fully started up.
+pid = lldbutil.wait_for_file_on_target(self, pid_file_path)
+
+# Now we can safely remove the executable and test if we can attach.
 os.remove(exe)
 
 self.runCmd("process attach -p " + str(popen.pid))


Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp
@@ -1,9 +1,15 @@
 #include 
+#include 
 #include 
 
-int main(int argc, char const *argv[])
-{
+int main(int argc, char const *argv[]) {
 lldb_enable_attach();
 
-std::this_thread::sleep_for(std::chrono::seconds(30));
+{
+  // Create file to signal that this process has started up.
+  std::ofstream f;
+  f.open(argv[1]);
+}
+
+std::this_thread::sleep_for(std::chrono::seconds(60));
 }
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
@@ -1,5 +1,5 @@
 """
-Test process attach/resume.
+Test process attach when executable was deleted.
 """
 
 from __future__ import print_function
@@ -24,8 +24,22 @@
 self.build()
 exe = self.getBuildArtifact("a.out")
 
-popen = self.spawnSubprocess(exe)
+# Use a file as a synchronization point between test and inferior.
+pid_file_path = lldbutil.append_to_process_working_directory(self,
+"token_pid_%d" % (int(os.getpid(
+self.addTearDownHook(
+lambda: self.run_platform_command(
+"rm %s" %
+(pid_file_path)))
+
+# Spawn a new process
+popen = self.spawnSubprocess(exe, [pid_file_path])
 self.addTearDownHook(self.cleanupSubprocesses)
+
+# Wait until process has fully started up.
+pid = lldbutil.wait_for_file_on_target(self, pid_file_path)
+
+# Now we can safely remove the executable and test if we can 

[Lldb-commits] [PATCH] D64545: [lldb] Don't use __FUNCTION__ as a file name

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365812: [lldb] Dont use __FUNCTION__ as a file name 
(authored by teemperor, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64545?vs=209124=209298#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64545

Files:
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp


Index: 
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -920,7 +920,7 @@
 
   if (!created_main_file) {
 std::unique_ptr memory_buffer =
-MemoryBuffer::getMemBufferCopy(expr_text, __FUNCTION__);
+MemoryBuffer::getMemBufferCopy(expr_text, "");
 
source_mgr.setMainFileID(source_mgr.createFileID(std::move(memory_buffer)));
   }
 


Index: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -920,7 +920,7 @@
 
   if (!created_main_file) {
 std::unique_ptr memory_buffer =
-MemoryBuffer::getMemBufferCopy(expr_text, __FUNCTION__);
+MemoryBuffer::getMemBufferCopy(expr_text, "");
 source_mgr.setMainFileID(source_mgr.createFileID(std::move(memory_buffer)));
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r365813 - [lldb] Make TestDeletedExecutable more reliable

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Thu Jul 11 12:27:33 2019
New Revision: 365813

URL: http://llvm.org/viewvc/llvm-project?rev=365813=rev
Log:
[lldb] Make TestDeletedExecutable more reliable

Summary:
It seems that calling Popen can return to the caller before the started process 
has read all the needed information
from its executable. This means that in case we delete the executable while the 
process is still starting up,
this test will create a zombie process which in turn leads to a failing test. 
On my macOS system this happens quite frequently.

This patch fixes this by letting the test synchronize with the inferior after 
it has started up.

Reviewers: davide

Reviewed By: davide

Subscribers: labath, friss, lldb-commits

Tags: #lldb

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py?rev=365813=365812=365813=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
 Thu Jul 11 12:27:33 2019
@@ -1,5 +1,5 @@
 """
-Test process attach/resume.
+Test process attach when executable was deleted.
 """
 
 from __future__ import print_function
@@ -24,8 +24,22 @@ class TestDeletedExecutable(TestBase):
 self.build()
 exe = self.getBuildArtifact("a.out")
 
-popen = self.spawnSubprocess(exe)
+# Use a file as a synchronization point between test and inferior.
+pid_file_path = lldbutil.append_to_process_working_directory(self,
+"token_pid_%d" % (int(os.getpid(
+self.addTearDownHook(
+lambda: self.run_platform_command(
+"rm %s" %
+(pid_file_path)))
+
+# Spawn a new process
+popen = self.spawnSubprocess(exe, [pid_file_path])
 self.addTearDownHook(self.cleanupSubprocesses)
+
+# Wait until process has fully started up.
+pid = lldbutil.wait_for_file_on_target(self, pid_file_path)
+
+# Now we can safely remove the executable and test if we can attach.
 os.remove(exe)
 
 self.runCmd("process attach -p " + str(popen.pid))

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp?rev=365813=365812=365813=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp
 Thu Jul 11 12:27:33 2019
@@ -1,9 +1,15 @@
 #include 
+#include 
 #include 
 
-int main(int argc, char const *argv[])
-{
+int main(int argc, char const *argv[]) {
 lldb_enable_attach();
 
-std::this_thread::sleep_for(std::chrono::seconds(30));
+{
+  // Create file to signal that this process has started up.
+  std::ofstream f;
+  f.open(argv[1]);
+}
+
+std::this_thread::sleep_for(std::chrono::seconds(60));
 }


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

[Lldb-commits] [lldb] r365812 - [lldb] Don't use __FUNCTION__ as a file name

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Thu Jul 11 12:26:55 2019
New Revision: 365812

URL: http://llvm.org/viewvc/llvm-project?rev=365812=rev
Log:
[lldb] Don't use __FUNCTION__ as a file name

Summary:
I saw while debugging that we call this file `ParseInternal`, which is not a 
very good name for our
fake expression file and also adds this unnecessary link between the way we 
name this function
and the other source location names we get from the expression parser. This 
patch is renaming
it to `` which is closer to the way Clang names its buffers, it 
doesn't depend on the
function name (which changes when I refactor this code) and it's easier to grep 
for.

Reviewers: davide

Reviewed By: davide

Subscribers: abidh, lldb-commits

Tags: #lldb

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

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

Modified: 
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp?rev=365812=365811=365812=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
Thu Jul 11 12:26:55 2019
@@ -920,7 +920,7 @@ ClangExpressionParser::ParseInternal(Dia
 
   if (!created_main_file) {
 std::unique_ptr memory_buffer =
-MemoryBuffer::getMemBufferCopy(expr_text, __FUNCTION__);
+MemoryBuffer::getMemBufferCopy(expr_text, "");
 
source_mgr.setMainFileID(source_mgr.createFileID(std::move(memory_buffer)));
   }
 


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

[Lldb-commits] [PATCH] D64536: Adding inline comments to code view type records

[NILANJANA BASU via Phabricator via lldb-commits]

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



Comment at: llvm/include/llvm/DebugInfo/CodeView/CodeViewRecordIO.h:63
 
-  Error mapInteger(TypeIndex );
+  Error mapInteger(TypeIndex , const char* Comment = nullptr);
 

rnk wrote:
> Is it possible to change these all to accept `const Twine  = ""`? I 
> think string literals are meant to be implicitly convertible to Twine.
> 
> Generally in LLVM StringRef is preferred over const char *. We have some 
> documentation for this here:
> https://llvm.org/docs/ProgrammersManual.html#passing-strings-the-stringref-and-twine-classes
> 
> I think for this API where the string length is unlikely to be used at all, 
> Twine makes sense, but across LLVM C strings are generally avoided if 
> possible.
Got it. Have made the changes & updated the patch.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D64536



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

[Lldb-commits] [PATCH] D64536: Adding inline comments to code view type records

[NILANJANA BASU via Phabricator via lldb-commits]

nilanjana_basu updated this revision to Diff 209278.
nilanjana_basu added a comment.
Herald added subscribers: lldb-commits, abidh.
Herald added a project: LLDB.

String parameter passing is done using Twine instead of using C strings, as 
suggested in the review


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D64536

Files:
  llvm/include/llvm/DebugInfo/CodeView/CodeViewRecordIO.h
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/DebugInfo/CodeView/CodeViewRecordIO.cpp
  llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp
  llvm/test/DebugInfo/COFF/types-basic.ll

Index: llvm/test/DebugInfo/COFF/types-basic.ll
===
--- llvm/test/DebugInfo/COFF/types-basic.ll
+++ llvm/test/DebugInfo/COFF/types-basic.ll
@@ -347,17 +347,15 @@
 ; CHECK:   ]
 ; CHECK: ]
 
-
-
 ; ASM: .section	.debug$T,"dr"
 ; ASM: .p2align	2
 ; ASM: .long	4   # Debug section magic
 ; ASM: .short	18
 ; ASM: .short	4609
-; ASM: .long	3
-; ASM: .long	64
-; ASM: .long	65
-; ASM: .long	19
+; ASM: .long	3   # NumArgs
+; ASM: .long	64  # Argument
+; ASM: .long	65  # Argument
+; ASM: .long	19  # Argument
 ; ASM: # ArgList (0x1000) {
 ; ASM: #   TypeLeafKind: LF_ARGLIST (0x1201)
 ; ASM: #   NumArgs: 3
@@ -369,11 +367,11 @@
 ; ASM: # }
 ; ASM: .short	14
 ; ASM: .short	4104
-; ASM: .long	3
-; ASM: .byte	0
-; ASM: .byte	0
-; ASM: .short	3
-; ASM: .long	4096
+; ASM: .long	3   # ReturnType
+; ASM: .byte	0   # CallingConvention
+; ASM: .byte	0   # FunctionOptions
+; ASM: .short	3   # NumParameters
+; ASM: .long	4096# ArgListType
 ; ASM: # Procedure (0x1001) {
 ; ASM: #   TypeLeafKind: LF_PROCEDURE (0x1008)
 ; ASM: #   ReturnType: void (0x3)
@@ -385,9 +383,9 @@
 ; ASM: # }
 ; ASM: .short	14
 ; ASM: .short	5633
-; ASM: .long	0
-; ASM: .long	4097
-; ASM: .asciz	"f"
+; ASM: .long	0   # ParentScope
+; ASM: .long	4097# FunctionType
+; ASM: .asciz	"f" # Name
 ; ASM: .byte	242
 ; ASM: .byte	241
 ; ASM: # FuncId (0x1002) {
@@ -398,8 +396,8 @@
 ; ASM: # }
 ; ASM: .short	10
 ; ASM: .short	4097
-; ASM: .long	116
-; ASM: .short	1
+; ASM: .long	116 # ModifiedType
+; ASM: .short	1   # Modifiers
 ; ASM: .byte	242
 ; ASM: .byte	241
 ; ASM: # Modifier (0x1003) {
@@ -411,8 +409,8 @@
 ; ASM: # }
 ; ASM: .short	10
 ; ASM: .short	4098
-; ASM: .long	4099
-; ASM: .long	65548
+; ASM: .long	4099# PointeeType
+; ASM: .long	65548   # Attributes
 ; ASM: # Pointer (0x1004) {
 ; ASM: #   TypeLeafKind: LF_POINTER (0x1002)
 ; ASM: #   PointeeType: const int (0x1003)
@@ -429,13 +427,13 @@
 ; ASM: # }
 ; ASM: .short	22
 ; ASM: .short	5381
-; ASM: .short	0
-; ASM: .short	128
-; ASM: .long	0
-; ASM: .long	0
-; ASM: .long	0
-; ASM: .short	0
-; ASM: .asciz	"A"
+; ASM: .short	0   # MemberCount
+; ASM: .short	128 # Properties
+; ASM: .long	0   # FieldList
+; ASM: .long	0   # DerivedFrom
+; ASM: .long	0   # VShape
+; ASM: .short	0   # SizeOf
+; ASM: .asciz	"A" # Name
 ; ASM: # Struct (0x1005) {
 ; ASM: #   TypeLeafKind: LF_STRUCTURE (0x1505)
 ; ASM: #   MemberCount: 0
@@ -450,10 +448,10 @@
 ; ASM: # }
 ; ASM: .short	18
 ; ASM: .short	4098
-; ASM: .long	116
-; ASM: .long	32844
-; ASM: .long	4101
-; ASM: .short	4
+; ASM: .long	116 # PointeeType
+; ASM: .long	32844   # Attributes
+; ASM: .long	4101# ClassType
+; ASM: .short	4   # Representation
 ; ASM: .byte	242
 ; ASM: .byte	241
 ; ASM: # Pointer (0x1006) {
@@ -474,8 +472,8 @@
 ; ASM: # }
 ; ASM: .short	10
 ; ASM: .short	4098
-; ASM: .long	4101
-; ASM: .long	66572
+; ASM: .long	4101# PointeeType
+; ASM: .long	66572   # Attributes
 ; ASM: # Pointer (0x1007) {
 ; ASM: #   TypeLeafKind: LF_POINTER (0x1002)
 ; ASM: #   PointeeType: A (0x1005)
@@ -492,7 +490,7 @@
 ; ASM: # }
 ; ASM: .short	6
 ; ASM: .short	4609
-; ASM: .long	0
+; ASM: .long	0   # NumArgs
 ; ASM: # ArgList (0x1008) {
 ; ASM: #   TypeLeafKind: LF_ARGLIST (0x1201)
 ; ASM: #   NumArgs: 0
@@ -501,14 +499,14 @@
 ; ASM: # }
 ; ASM: .short	26
 ; ASM: .short	4105
-; ASM: .long	3
-; ASM: .long	4101
-; ASM: .long	4103
-; ASM: .byte	0
-; ASM: .byte	0
-; ASM: .short	0
-; ASM: .long	4104
-; ASM: .long	0
+; ASM: .long	3   # ReturnType
+; ASM: .long	4101# ClassType
+; ASM: .long	4103# ThisType
+; ASM: .byte	0   # CallingConvention
+; ASM: .byte	0   # FunctionOptions
+; 

[Lldb-commits] [PATCH] D64546: [lldb] Make TestDeletedExecutable more reliable

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 209231.
teemperor edited the summary of this revision.
teemperor added a comment.

- Timeout -> file synchronization.
- Made the test process run longer just in case.


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

https://reviews.llvm.org/D64546

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
  
lldb/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp


Index: 
lldb/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp
===
--- 
lldb/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp
+++ 
lldb/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp
@@ -1,9 +1,15 @@
 #include 
+#include 
 #include 
 
-int main(int argc, char const *argv[])
-{
+int main(int argc, char const *argv[]) {
 lldb_enable_attach();
 
-std::this_thread::sleep_for(std::chrono::seconds(30));
+{
+  // Create file to signal that this process has started up.
+  std::ofstream f;
+  f.open(argv[1]);
+}
+
+std::this_thread::sleep_for(std::chrono::seconds(60));
 }
Index: 
lldb/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
===
--- 
lldb/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
+++ 
lldb/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
@@ -1,5 +1,5 @@
 """
-Test process attach/resume.
+Test process attach when executable was deleted.
 """
 
 from __future__ import print_function
@@ -24,8 +24,22 @@
 self.build()
 exe = self.getBuildArtifact("a.out")
 
-popen = self.spawnSubprocess(exe)
+# Use a file as a synchronization point between test and inferior.
+pid_file_path = lldbutil.append_to_process_working_directory(self,
+"token_pid_%d" % (int(os.getpid(
+self.addTearDownHook(
+lambda: self.run_platform_command(
+"rm %s" %
+(pid_file_path)))
+
+# Spawn a new process
+popen = self.spawnSubprocess(exe, [pid_file_path])
 self.addTearDownHook(self.cleanupSubprocesses)
+
+# Wait until process has fully started up.
+pid = lldbutil.wait_for_file_on_target(self, pid_file_path)
+
+# Now we can safely remove the executable and test if we can attach.
 os.remove(exe)
 
 self.runCmd("process attach -p " + str(popen.pid))


Index: lldb/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp
===
--- lldb/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp
+++ lldb/packages/Python/lldbsuite/test/functionalities/deleted-executable/main.cpp
@@ -1,9 +1,15 @@
 #include 
+#include 
 #include 
 
-int main(int argc, char const *argv[])
-{
+int main(int argc, char const *argv[]) {
 lldb_enable_attach();
 
-std::this_thread::sleep_for(std::chrono::seconds(30));
+{
+  // Create file to signal that this process has started up.
+  std::ofstream f;
+  f.open(argv[1]);
+}
+
+std::this_thread::sleep_for(std::chrono::seconds(60));
 }
Index: lldb/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/deleted-executable/TestDeletedExecutable.py
@@ -1,5 +1,5 @@
 """
-Test process attach/resume.
+Test process attach when executable was deleted.
 """
 
 from __future__ import print_function
@@ -24,8 +24,22 @@
 self.build()
 exe = self.getBuildArtifact("a.out")
 
-popen = self.spawnSubprocess(exe)
+# Use a file as a synchronization point between test and inferior.
+pid_file_path = lldbutil.append_to_process_working_directory(self,
+"token_pid_%d" % (int(os.getpid(
+self.addTearDownHook(
+lambda: self.run_platform_command(
+"rm %s" %
+(pid_file_path)))
+
+# Spawn a new process
+popen = self.spawnSubprocess(exe, [pid_file_path])
 self.addTearDownHook(self.cleanupSubprocesses)
+
+# Wait until process has fully started up.
+pid = lldbutil.wait_for_file_on_target(self, pid_file_path)
+
+# Now we can safely remove the executable and test if we can attach.
 os.remove(exe)
 
 self.runCmd("process attach -p " + str(popen.pid))
___
lldb-commits mailing list
lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D64546: [lldb] Make TestDeletedExecutable more reliable

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D64546#1580618 , @friss wrote:

> This might mitigate the issue, but timeouts like this are bound to fail in 
> some circumstances (machine load, ...). It's more work, but can we instead 
> have the inferior produce an observable side effect (eg, print some output) 
> and synchronize on this?


There's a `lldbutil.wait_for_file_on_target`, which was created exactly for 
this purpose.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D64546



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

[Lldb-commits] [PATCH] D64546: [lldb] Make TestDeletedExecutable more reliable

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Yeah, I'd rather have an explicit communication between debugger and debuggee.
We tried to put sleeps in the code [for e.g. `lldb-mi` tests in the past] and 
they end up failing anyway sporadically, which makes a pain to track the 
problem down.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D64546



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

[Lldb-commits] [PATCH] D64546: [lldb] Make TestDeletedExecutable more reliable

[Frederic Riss via Phabricator via lldb-commits]

friss added a comment.

This might mitigate the issue, but timeouts like this are bound to fail in some 
circumstances (machine load, ...). It's more work, but can we instead have the 
inferior produce an observable side effect (eg, print some output) and 
synchronize on this?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D64546



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

[Lldb-commits] [PATCH] D64398: [LLDB] Fix FreeBSD build

[David CARLIER via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365761: [LLDB] Fix FreeBSD build. (authored by devnexen, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64398?vs=208649=209181#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64398

Files:
  lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
  lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h

Index: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h
===
--- lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h
+++ lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h
@@ -183,8 +183,8 @@
 private:
   ProcessFreeBSD *m_process;
 
-  lldb_private::HostThread m_operation_thread;
-  lldb_private::HostThread m_monitor_thread;
+  llvm::Expected m_operation_thread;
+  llvm::Expected m_monitor_thread;
   lldb::pid_t m_pid;
 
   int m_terminal_fd;
Index: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
===
--- lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
+++ lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
@@ -708,7 +708,7 @@
 const lldb_private::ProcessLaunchInfo & /* launch_info */,
 lldb_private::Status )
 : m_process(static_cast(process)),
-  m_pid(LLDB_INVALID_PROCESS_ID), m_terminal_fd(-1), m_operation(0) {
+  m_operation_thread(nullptr), m_monitor_thread(nullptr), m_pid(LLDB_INVALID_PROCESS_ID), m_terminal_fd(-1), m_operation(0) {
   using namespace std::placeholders;
 
   std::unique_ptr args(
@@ -738,7 +738,7 @@
   m_monitor_thread = Host::StartMonitoringChildProcess(
   std::bind(::MonitorCallback, this, _1, _2, _3, _4),
   GetPID(), true);
-  if (!m_monitor_thread.IsJoinable()) {
+  if (!m_monitor_thread->IsJoinable()) {
 error.SetErrorToGenericError();
 error.SetErrorString("Process launch failed.");
 return;
@@ -747,8 +747,8 @@
 
 ProcessMonitor::ProcessMonitor(ProcessFreeBSD *process, lldb::pid_t pid,
lldb_private::Status )
-: m_process(static_cast(process)), m_pid(pid),
-  m_terminal_fd(-1), m_operation(0) {
+: m_process(static_cast(process)),
+  m_operation_thread(nullptr), m_monitor_thread(nullptr), m_pid(pid), m_terminal_fd(-1), m_operation(0) {
   using namespace std::placeholders;
 
   sem_init(_operation_pending, 0, 0);
@@ -776,7 +776,7 @@
   m_monitor_thread = Host::StartMonitoringChildProcess(
   std::bind(::MonitorCallback, this, _1, _2, _3, _4),
   GetPID(), true);
-  if (!m_monitor_thread.IsJoinable()) {
+  if (!m_monitor_thread->IsJoinable()) {
 error.SetErrorToGenericError();
 error.SetErrorString("Process attach failed.");
 return;
@@ -789,11 +789,13 @@
 void ProcessMonitor::StartLaunchOpThread(LaunchArgs *args, Status ) {
   static const char *g_thread_name = "lldb.process.freebsd.operation";
 
-  if (m_operation_thread.IsJoinable())
+  if (m_operation_thread->IsJoinable())
 return;
 
   m_operation_thread =
-  ThreadLauncher::LaunchThread(g_thread_name, LaunchOpThread, args, );
+  ThreadLauncher::LaunchThread(g_thread_name, LaunchOpThread, args);
+  if (!m_operation_thread)
+ error = m_operation_thread.takeError();
 }
 
 void *ProcessMonitor::LaunchOpThread(void *arg) {
@@ -955,11 +957,14 @@
  lldb_private::Status ) {
   static const char *g_thread_name = "lldb.process.freebsd.operation";
 
-  if (m_operation_thread.IsJoinable())
+  if (m_operation_thread->IsJoinable())
 return;
 
   m_operation_thread =
-  ThreadLauncher::LaunchThread(g_thread_name, AttachOpThread, args, );
+  ThreadLauncher::LaunchThread(g_thread_name, AttachOpThread, args);
+
+  if (!m_operation_thread)
+	error = m_operation_thread.takeError();
 }
 
 void *ProcessMonitor::AttachOpThread(void *arg) {
@@ -1379,10 +1384,10 @@
 }
 
 void ProcessMonitor::StopMonitoringChildProcess() {
-  if (m_monitor_thread.IsJoinable()) {
-m_monitor_thread.Cancel();
-m_monitor_thread.Join(nullptr);
-m_monitor_thread.Reset();
+  if (m_monitor_thread->IsJoinable()) {
+m_monitor_thread->Cancel();
+m_monitor_thread->Join(nullptr);
+m_monitor_thread->Reset();
   }
 }
 
@@ -1417,10 +1422,10 @@
 bool ProcessMonitor::WaitForInitialTIDStop(lldb::tid_t tid) { return true; }
 
 void ProcessMonitor::StopOpThread() {
-  if (!m_operation_thread.IsJoinable())
+  if (!m_operation_thread->IsJoinable())
 return;
 
-  m_operation_thread.Cancel();
-  m_operation_thread.Join(nullptr);
-  m_operation_thread.Reset();
+  m_operation_thread->Cancel();
+  m_operation_thread->Join(nullptr);
+  m_operation_thread->Reset();
 }
___
lldb-commits mailing list

[Lldb-commits] [lldb] r365761 - [LLDB] Fix FreeBSD build.

[David Carlier via lldb-commits]

Author: devnexen
Date: Thu Jul 11 05:21:04 2019
New Revision: 365761

URL: http://llvm.org/viewvc/llvm-project?rev=365761=rev
Log:
[LLDB] Fix FreeBSD build.

To align with the LaunchThread change.

Reviewers: MaskRay, mgorny

Reviewed By: MaskRay

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

Modified:
lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h

Modified: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp?rev=365761=365760=365761=diff
==
--- lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp (original)
+++ lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp Thu Jul 11 
05:21:04 2019
@@ -708,7 +708,7 @@ ProcessMonitor::ProcessMonitor(
 const lldb_private::ProcessLaunchInfo & /* launch_info */,
 lldb_private::Status )
 : m_process(static_cast(process)),
-  m_pid(LLDB_INVALID_PROCESS_ID), m_terminal_fd(-1), m_operation(0) {
+  m_operation_thread(nullptr), m_monitor_thread(nullptr), 
m_pid(LLDB_INVALID_PROCESS_ID), m_terminal_fd(-1), m_operation(0) {
   using namespace std::placeholders;
 
   std::unique_ptr args(
@@ -738,7 +738,7 @@ ProcessMonitor::ProcessMonitor(
   m_monitor_thread = Host::StartMonitoringChildProcess(
   std::bind(::MonitorCallback, this, _1, _2, _3, _4),
   GetPID(), true);
-  if (!m_monitor_thread.IsJoinable()) {
+  if (!m_monitor_thread->IsJoinable()) {
 error.SetErrorToGenericError();
 error.SetErrorString("Process launch failed.");
 return;
@@ -747,8 +747,8 @@ ProcessMonitor::ProcessMonitor(
 
 ProcessMonitor::ProcessMonitor(ProcessFreeBSD *process, lldb::pid_t pid,
lldb_private::Status )
-: m_process(static_cast(process)), m_pid(pid),
-  m_terminal_fd(-1), m_operation(0) {
+: m_process(static_cast(process)),
+  m_operation_thread(nullptr), m_monitor_thread(nullptr), m_pid(pid), 
m_terminal_fd(-1), m_operation(0) {
   using namespace std::placeholders;
 
   sem_init(_operation_pending, 0, 0);
@@ -776,7 +776,7 @@ ProcessMonitor::ProcessMonitor(ProcessFr
   m_monitor_thread = Host::StartMonitoringChildProcess(
   std::bind(::MonitorCallback, this, _1, _2, _3, _4),
   GetPID(), true);
-  if (!m_monitor_thread.IsJoinable()) {
+  if (!m_monitor_thread->IsJoinable()) {
 error.SetErrorToGenericError();
 error.SetErrorString("Process attach failed.");
 return;
@@ -789,11 +789,13 @@ ProcessMonitor::~ProcessMonitor() { Stop
 void ProcessMonitor::StartLaunchOpThread(LaunchArgs *args, Status ) {
   static const char *g_thread_name = "lldb.process.freebsd.operation";
 
-  if (m_operation_thread.IsJoinable())
+  if (m_operation_thread->IsJoinable())
 return;
 
   m_operation_thread =
-  ThreadLauncher::LaunchThread(g_thread_name, LaunchOpThread, args, 
);
+  ThreadLauncher::LaunchThread(g_thread_name, LaunchOpThread, args);
+  if (!m_operation_thread)
+ error = m_operation_thread.takeError();
 }
 
 void *ProcessMonitor::LaunchOpThread(void *arg) {
@@ -955,11 +957,14 @@ void ProcessMonitor::StartAttachOpThread
  lldb_private::Status ) {
   static const char *g_thread_name = "lldb.process.freebsd.operation";
 
-  if (m_operation_thread.IsJoinable())
+  if (m_operation_thread->IsJoinable())
 return;
 
   m_operation_thread =
-  ThreadLauncher::LaunchThread(g_thread_name, AttachOpThread, args, 
);
+  ThreadLauncher::LaunchThread(g_thread_name, AttachOpThread, args);
+
+  if (!m_operation_thread)
+   error = m_operation_thread.takeError();
 }
 
 void *ProcessMonitor::AttachOpThread(void *arg) {
@@ -1379,10 +1384,10 @@ bool ProcessMonitor::DupDescriptor(const
 }
 
 void ProcessMonitor::StopMonitoringChildProcess() {
-  if (m_monitor_thread.IsJoinable()) {
-m_monitor_thread.Cancel();
-m_monitor_thread.Join(nullptr);
-m_monitor_thread.Reset();
+  if (m_monitor_thread->IsJoinable()) {
+m_monitor_thread->Cancel();
+m_monitor_thread->Join(nullptr);
+m_monitor_thread->Reset();
   }
 }
 
@@ -1417,10 +1422,10 @@ void ProcessMonitor::StopMonitor() {
 bool ProcessMonitor::WaitForInitialTIDStop(lldb::tid_t tid) { return true; }
 
 void ProcessMonitor::StopOpThread() {
-  if (!m_operation_thread.IsJoinable())
+  if (!m_operation_thread->IsJoinable())
 return;
 
-  m_operation_thread.Cancel();
-  m_operation_thread.Join(nullptr);
-  m_operation_thread.Reset();
+  m_operation_thread->Cancel();
+  m_operation_thread->Join(nullptr);
+  m_operation_thread->Reset();
 }

Modified: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h?rev=365761=365760=365761=diff

[Lldb-commits] [PATCH] D64398: [LLDB] Fix FreeBSD build

[Fangrui Song via Phabricator via lldb-commits]

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

I think the description should mention that this is to fix FreeBSD build after 
the LaunchThread interface change.


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

https://reviews.llvm.org/D64398



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

[Lldb-commits] [PATCH] D64398: [LLDB] Fix FreeBSD build

[David CARLIER via Phabricator via lldb-commits]

devnexen added a comment.

ping :)


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

https://reviews.llvm.org/D64398



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