[Lldb-commits] [PATCH] D92264: [lldb] [POSIX-DYLD] Update the cached exe path after attach

2020-12-10 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

In D92264#2436839 , @mgorny wrote:

> Rewritten the test wrt comments, that is:
>
> 1. Added a breakpoint-continue to ensure that the program is past all initial 
> work before we test it.

I am not sure that will help -- I'm guessing the dynamic loader plugin will go 
through a substantially different codepath if the attach happens early on vs. 
if it happens after main is reached. It may be better to update this to do the 
`wait_for_file_on_target` dance, which would also prevent attach denied errors.




Comment at: lldb/test/API/commands/process/attach/TestProcessAttach.py:94
+
+self.runCmd("process attach -p " + str(popen.pid))
+

mgorny wrote:
> labath wrote:
> > Do you need to attach at any particular moment during the process startup? 
> > The attach command could run pretty quickly, before the loader finished its 
> > work...
> I don't know ;-). Do you have any particular suggestion? That said, I suppose 
> this means other tests suffer from the same problem, in particular they could 
> attach before `lldb_enable_attach()`.
Good point. Some of our attach tests have the additional 
`lldbutil.wait_for_file_on_target` call as an additional synchronization. I 
guess this test is not one of them...


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

https://reviews.llvm.org/D92264

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


[Lldb-commits] [PATCH] D82857: [LLDB] Add per-thread register infos shared pointer in gdb-remote

2020-12-10 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

looks good


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

https://reviews.llvm.org/D82857

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


[Lldb-commits] [PATCH] D92187: [lldb] [POSIX-DYLD] Add libraries from initial rendezvous brkpt hit

2020-12-10 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

Actually, I can still reproduce the double symbol problem with Arch's 
`ld-2.32.so`. I can't figure out what's wrong.


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

https://reviews.llvm.org/D92187

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


[Lldb-commits] [PATCH] D92187: [lldb] [POSIX-DYLD] Add libraries from initial rendezvous brkpt hit

2020-12-10 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

One possible hackaround would be to do this only if 1st breakpoint hit is 
consistent (vs add on Linux), or just make the whole logic `#if 
defined(__FreeBSD__)`.


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

https://reviews.llvm.org/D92187

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


[Lldb-commits] [PATCH] D82866: [LLDB] Test SVE dynamic resize with multiple threads

2020-12-10 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c:58
+void *thread_func(void *x_void_ptr) {
+  if (*((int *)x_void_ptr) == 0) {
+prctl(PR_SVE_SET_VL, 8 * 4);

It looks like it would be simpler if these were two functions.


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

https://reviews.llvm.org/D82866

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


[Lldb-commits] [PATCH] D82863: [LLDB] Add support to resize SVE registers at run-time

2020-12-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp:792-793
+  }
+  reg.byte_offset = end_reg_offset;
+  end_reg_offset += reg.byte_size;
+}

We already have a mostly generic algorithm for computing the initial offsets of 
registers. This code reintroduces the assumptions about the ordering of 
registers and similar.

 It would be nice if this could reuse that offset computation code (after 
updating the register sizes).  One possibility might be to stash the original 
register infos (with empty byte offsets) somewhere, and then reinitialize this 
register context from *that*. 



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp:767
+  m_reg_data.Clear();
+  m_reg_data.SetData(reg_data_sp);
+  m_reg_data.SetByteOrder(GetByteOrder());

omjavaid wrote:
> jasonmolenda wrote:
> > I'm probably not following this correctly, but isn't this going to shorten 
> > the register buffer m_reg_data to the end of the SVE registers in the 
> > buffer, right?  If the goal here is to create a heap object, why not just 
> > copy the entire m_data_reg?  If someone ever adds a register past the 
> > SVE's, this would truncate it away.  
> Register data can expand on shrink based on vector length update of SVE 
> register set. So heap size will change accordingly.
> We need to preserve VG register and GPRs from current register data so we 
> copy the whole data in case vector length update caused register data to 
> expand. However we truncate data to new vector length in case vector length 
> has shrunk. All register past VG registers are invalidated anyways. 
How often do you expect the user to be changing the vector length? Could we 
just create a completely empty (invalid) buffer of the right size and force a 
re-fetch the next time the registers are accessed? With `g` packets, this 
should be a single round-trip anyway...


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

https://reviews.llvm.org/D82863

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


[Lldb-commits] [lldb] 4df4edb - [lldb][NFC] Fix a typo in TestCppMultipleInheritance

2020-12-10 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-12-10T10:56:46+01:00
New Revision: 4df4edb6ad14c7748acda8670944cde19d537621

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

LOG: [lldb][NFC] Fix a typo in TestCppMultipleInheritance

Added: 


Modified: 
lldb/test/API/lang/cpp/multiple-inheritance/TestCppMultipleInheritance.py

Removed: 




diff  --git 
a/lldb/test/API/lang/cpp/multiple-inheritance/TestCppMultipleInheritance.py 
b/lldb/test/API/lang/cpp/multiple-inheritance/TestCppMultipleInheritance.py
index defd4bd5df5f..bc2b7a5f95b0 100644
--- a/lldb/test/API/lang/cpp/multiple-inheritance/TestCppMultipleInheritance.py
+++ b/lldb/test/API/lang/cpp/multiple-inheritance/TestCppMultipleInheritance.py
@@ -9,7 +9,7 @@ class TestCase(TestBase):
 
 def test(self):
 self.build()
-lldbutil.run_to_source_breakpoint(self,"// break here", 
lldb.SBFileSpec("main.cpp"))
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"))
 
 # Member access
 self.expect_expr("C.Base1::m_base", result_type="int", 
result_value="11")



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


[Lldb-commits] [PATCH] D92643: [lldb] Lookup static const members in FindGlobalVariables

2020-12-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

It looks like the Type interface does not expose the static members (I guess it 
was never meant to do that), although, the underlying clang type obviously does 
contain them. After more consideration, I think parsing these straight from 
DWARF is fine, and consistent with how we parse other variables. I am wondering 
though if this should share the implementation with the existing parsing code 
(like your first patch did).

On a higher level, I am also worried about what this (the fact that we have to 
do this kind of a search) says about the quality/usefulness of the debug_names 
index. Overall, I seems to me that _not_ including these member variables in 
the index is the right call -- they would bloat the index (you'd have to put 
one of these next to every type declaration), and there are (as we've seen) 
other ways to find them.

I am not sure what to do about namespace-level constexpr variables, though... 
Currently, clang seems to put them into the index, even though that's not 
specified by standard. My guess is that this is by accident (it just inserts 
all global vars, without checking the formal inclusion rules). However, the 
question is what that means. If we were to consider this a bug and "fix" clang 
to stop emitting them, then we'd need to find a way to locate them in lldb. And 
doing this for namespaces won't be as easy/cheap as it is for classes. Maybe it 
doesn't have to block this patch (as I said, I think this part is ok), but I 
think it would be useful to discuss this with the people responsible for the 
generation of the index in clang and the wider dwarf community. (Unfortunately, 
I don't have the bandwidth to drive that dicussion :( ).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92643

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


[Lldb-commits] [lldb] 9586082 - [lldb] Allow LLDB to automatically retry a failed expression with an imported std C++ module

2020-12-10 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-12-10T12:29:17+01:00
New Revision: 958608285eb4d04c6e3dc7071fa429b43597585b

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

LOG: [lldb] Allow LLDB to automatically retry a failed expression with an 
imported std C++ module

By now LLDB can import the 'std' C++ module to improve expression evaluation,
but there are still a few problems to solve before we can do this by default.
One is that importing the C++ module is slightly slower than normal expression
evaluation (mostly because the disk access and loading the initial lookup data
is quite slow in comparison to the barebone Clang setup the rest of the LLDB
expression evaluator is usually doing). Another problem is that some complicated
types in the standard library aren't fully supported yet by the ASTImporter, so
we end up types that fail to import (which usually appears to the user as if the
type is empty or there is just no result variable).

To still allow people to adopt this mode in their daily debugging, this patch
adds a setting that allows LLDB to automatically retry failed expression with a
loaded C++ module. All success expressions will behave exactly as they would do
before this patch. Failed expressions get a another parse attempt if we find a
usable C++ module in the current execution context. This way we shouldn't have
any performance/parsing regressions in normal debugging workflows, while the
debugging workflows involving STL containers benefit from the C++ module type
info.

This setting is off by default for now with the intention to enable it by
default on macOS soon-ish.

The implementation is mostly just extracting the existing parse logic into its
own function and then calling the parse function again if the first evaluation
failed and we have a C++ module to retry the parsing with.

Reviewed By: shafik, JDevlieghere, aprantl

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

Added: 

lldb/test/API/commands/expression/import-std-module/retry-with-std-module/Makefile

lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py

lldb/test/API/commands/expression/import-std-module/retry-with-std-module/main.cpp

Modified: 
lldb/include/lldb/Target/Target.h
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td

Removed: 




diff  --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 47568c9e4429..69baefb964b0 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -65,6 +65,12 @@ enum LoadDependentFiles {
   eLoadDependentsNo,
 };
 
+enum ImportStdModule {
+  eImportStdModuleFalse,
+  eImportStdModuleFallback,
+  eImportStdModuleTrue,
+};
+
 class TargetExperimentalProperties : public Properties {
 public:
   TargetExperimentalProperties();
@@ -135,7 +141,7 @@ class TargetProperties : public Properties {
 
   bool GetEnableAutoImportClangModules() const;
 
-  bool GetEnableImportStdModule() const;
+  ImportStdModule GetImportStdModule() const;
 
   bool GetEnableAutoApplyFixIts() const;
 

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index a28b4a7fb42c..9be294750fa0 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -417,7 +417,6 @@ void ClangUserExpression::CreateSourceCode(
 DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
 std::vector modules_to_import, bool for_completion) {
 
-  m_filename = m_clang_state->GetNextExprFileName();
   std::string prefix = m_expr_prefix;
 
   if (m_options.GetExecutionPolicy() == eExecutionPolicyTopLevel) {
@@ -477,9 +476,6 @@ CppModuleConfiguration GetModuleConfig(lldb::LanguageType 
language,
   if (!target)
 return LogConfigError("No target");
 
-  if (!target->GetEnableImportStdModule())
-return LogConfigError("Importing std module not enabled in settings");
-
   StackFrame *frame = exe_ctx.GetFramePtr();
   if (!frame)
 return LogConfigError("No frame");
@@ -529,8 +525,6 @@ CppModuleConfiguration GetModuleConfig(lldb::LanguageType 
language,
 bool ClangUserExpression::PrepareForParsing(
 DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
 bool for_completion) {
-  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
-
   InstallContext(exe_ctx);
 
   if (!SetupPersistentState(diagnostic_manager, exe_ctx))
@@ -551,50 +545,20 @@ bool ClangUserExpression::PrepareForPars

[Lldb-commits] [PATCH] D92784: [lldb] Allow LLDB to automatically retry a failed expression with an import std C++ module

2020-12-10 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG958608285eb4: [lldb] Allow LLDB to automatically retry a 
failed expression with an imported… (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92784

Files:
  lldb/include/lldb/Target/Target.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  
lldb/test/API/commands/expression/import-std-module/retry-with-std-module/Makefile
  
lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py
  
lldb/test/API/commands/expression/import-std-module/retry-with-std-module/main.cpp

Index: lldb/test/API/commands/expression/import-std-module/retry-with-std-module/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/import-std-module/retry-with-std-module/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+int main(int argc, char **argv) {
+  std::vector a = {3, 1, 2};
+  int local = 3;
+  return 0; // Set break point at this line.
+}
Index: lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py
@@ -0,0 +1,76 @@
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@add_test_categories(["libc++"])
+@skipIf(compiler=no_match("clang"))
+def test(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self,
+  "// Set break point at this line.",
+  lldb.SBFileSpec("main.cpp"))
+
+# Test printing the vector before enabling any C++ module setting.
+self.expect_expr("a", result_type="std::vector >")
+
+# Set loading the import-std-module to 'fallback' which loads the module
+# and retries when an expression fails to parse.
+self.runCmd("settings set target.import-std-module fallback")
+
+# Printing the vector still works. This should return the same type
+# as before as this shouldn't use a C++ module type (the C++ module type
+# is hiding the second template parameter as it's equal to the default
+# argument which the C++ module has type info for).
+self.expect_expr("a", result_type="std::vector >")
+
+# This expression can only parse with a C++ module. LLDB should
+# automatically fall back to import the C++ module to get this working.
+self.expect_expr("std::max(0U, a.size())", result_value="3")
+
+
+# The 'a' and 'local' part can be parsed without loading a C++ module and will
+# load type/runtime information. The 'std::max...' part will fail to
+# parse without a C++ module. Make sure we reset all the relevant parts of
+# the C++ parser so that we don't end up with for example a second
+# definition of 'local' when retrying.
+self.expect_expr("a; local; std::max(0U, a.size())", result_value="3")
+
+
+# Try to declare top-level declarations that require a C++ module to parse.
+# Top-level expressions don't support importing the C++ module (yet), so
+# this should still fail as before.
+self.expect("expr --top-level -- int i = std::max(1, 2);", error=True,
+substrs=["no member named 'max' in namespace 'std'"])
+
+# Check that diagnostics from the first parse attempt don't show up
+# in the C++ module parse attempt. In the expression below, we first
+# fail to parse 'std::max'. Then we retry with a loaded C++ module
+# and succeed to parse the 'std::max' part. However, the
+# trailing 'unknown_identifier' will fail to parse even with the
+# loaded module. The 'std::max' diagnostic from the first attempt
+# however should not be shown to the user.
+self.expect("expr std::max(1, 2); unknown_identifier", error=True,
+matching=False,
+substrs=["no member named 'max'"])
+# The proper diagnostic however should be shown on the retry.
+self.expect("expr std::max(1, 2); unknown_identifier", error=True,
+substrs=["use of undeclared identifier 'unknown_identifier'"])
+
+# Turn on the 'import-std-module' setting and make sure we import the
+# C++ module.
+self.runCmd("set

[Lldb-commits] [PATCH] D92643: [lldb] Lookup static const members in FindGlobalVariables

2020-12-10 Thread Andy Yankovsky via Phabricator via lldb-commits
werat added a comment.

Thanks for your review! If there are no objections to this patch, can you 
accept and land it for me? :)

In D92643#2445146 , @labath wrote:

> On a higher level, I am also worried about what this (the fact that we have 
> to do this kind of a search) says about the quality/usefulness of the 
> debug_names index.

FWIW we're considering disabling `debug_names` completely for now, as it 
degrades performance significantly with no extra benefit. We can discuss that 
in more details if you're interested.

In D92643#2445146 , @labath wrote:

> I think it would be useful to discuss this with the people responsible for 
> the generation of the index in clang and the wider dwarf community. 
> (Unfortunately, I don't have the bandwidth to drive that dicussion :( ).

Can't say I have a lot of expertise in DWARF, but I will try to research this 
more and reach out to clang/dwarf people.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92643

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


[Lldb-commits] [lldb] 208e3f5 - [lldb] Fix that symbols.clang-modules-cache-path is never initialized

2020-12-10 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-12-10T13:37:40+01:00
New Revision: 208e3f5d9b6c172f65dbb9cdbc9354c81c6d8911

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

LOG: [lldb] Fix that symbols.clang-modules-cache-path is never initialized

LLDB is supposed to ask the Clang Driver what the default module cache path is
and then use that value as the default for the
`symbols.clang-modules-cache-path` setting. However, we use the property type
`String` to change `symbols.clang-modules-cache-path` even though the type of
that setting is `FileSpec`, so the setter will simply do nothing and return
`false`. We also don't check the return value of the setter, so this whole code
ends up not doing anything at all.

This changes the setter to use the correct property type and adds an assert that
we actually successfully set the default path. Also adds a test that checks that
the default value for this setting is never unset/empty path as this would
effectively disable the import-std-module feature from working by default.

Reviewed By: JDevlieghere, shafik

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

Added: 
lldb/test/Shell/Settings/TestDefaultModuleCachePath.test

Modified: 
lldb/include/lldb/Core/ModuleList.h
lldb/source/Core/ModuleList.cpp
lldb/test/Shell/helper/toolchain.py

Removed: 




diff  --git a/lldb/include/lldb/Core/ModuleList.h 
b/lldb/include/lldb/Core/ModuleList.h
index d90b27e474ac..1609f0f77c56 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -56,7 +56,7 @@ class ModuleListProperties : public Properties {
   ModuleListProperties();
 
   FileSpec GetClangModulesCachePath() const;
-  bool SetClangModulesCachePath(llvm::StringRef path);
+  bool SetClangModulesCachePath(const FileSpec &path);
   bool GetEnableExternalLookup() const;
   bool SetEnableExternalLookup(bool new_value);
 

diff  --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index cf276ba65931..b6e1ceb40889 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -82,8 +82,9 @@ ModuleListProperties::ModuleListProperties() {
[this] { UpdateSymlinkMappings(); 
});
 
   llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  if (clang::driver::Driver::getDefaultModuleCachePath(path)) {
+lldbassert(SetClangModulesCachePath(FileSpec(path)));
+  }
 }
 
 bool ModuleListProperties::GetEnableExternalLookup() const {
@@ -104,8 +105,8 @@ FileSpec ModuleListProperties::GetClangModulesCachePath() 
const {
   ->GetCurrentValue();
 }
 
-bool ModuleListProperties::SetClangModulesCachePath(llvm::StringRef path) {
-  return m_collection_sp->SetPropertyAtIndexAsString(
+bool ModuleListProperties::SetClangModulesCachePath(const FileSpec &path) {
+  return m_collection_sp->SetPropertyAtIndexAsFileSpec(
   nullptr, ePropertyClangModulesCachePath, path);
 }
 

diff  --git a/lldb/test/Shell/Settings/TestDefaultModuleCachePath.test 
b/lldb/test/Shell/Settings/TestDefaultModuleCachePath.test
new file mode 100644
index ..80dc0f25ce58
--- /dev/null
+++ b/lldb/test/Shell/Settings/TestDefaultModuleCachePath.test
@@ -0,0 +1,9 @@
+# RUN: %lldb-noinit -x -s %s | FileCheck %s
+settings show symbols.clang-modules-cache-path
+q
+# This test checks that we get *any* clang modules cache path by default. The
+# actual path depends on the operating system.
+# CHECK: symbols.clang-modules-cache-path (file) = "
+# Clang treats an empty path in the same way as 'no path', so explicitly check
+# that we never have an empty path by default.
+# CHECK-NOT: symbols.clang-modules-cache-path (file) = ""

diff  --git a/lldb/test/Shell/helper/toolchain.py 
b/lldb/test/Shell/helper/toolchain.py
index ebf9e03d81a0..59b078411c1c 100644
--- a/lldb/test/Shell/helper/toolchain.py
+++ b/lldb/test/Shell/helper/toolchain.py
@@ -54,6 +54,10 @@ def use_lldb_substitutions(config):
   command=FindTool('lldb'),
   extra_args=['-S', lldb_init],
   unresolved='fatal'),
+ToolSubst('%lldb-noinit',
+  command=FindTool('lldb'),
+  extra_args=['--no-lldbinit'],
+  unresolved='fatal'),
 ToolSubst('%lldb-server',
   command=FindTool("lldb-server"),
   extra_args=[],



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


[Lldb-commits] [PATCH] D92772: [lldb] Fix that symbols.clang-modules-cache-path is never initialized

2020-12-10 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG208e3f5d9b6c: [lldb] Fix that 
symbols.clang-modules-cache-path is never initialized (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92772

Files:
  lldb/include/lldb/Core/ModuleList.h
  lldb/source/Core/ModuleList.cpp
  lldb/test/Shell/Settings/TestDefaultModuleCachePath.test
  lldb/test/Shell/helper/toolchain.py


Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -54,6 +54,10 @@
   command=FindTool('lldb'),
   extra_args=['-S', lldb_init],
   unresolved='fatal'),
+ToolSubst('%lldb-noinit',
+  command=FindTool('lldb'),
+  extra_args=['--no-lldbinit'],
+  unresolved='fatal'),
 ToolSubst('%lldb-server',
   command=FindTool("lldb-server"),
   extra_args=[],
Index: lldb/test/Shell/Settings/TestDefaultModuleCachePath.test
===
--- /dev/null
+++ lldb/test/Shell/Settings/TestDefaultModuleCachePath.test
@@ -0,0 +1,9 @@
+# RUN: %lldb-noinit -x -s %s | FileCheck %s
+settings show symbols.clang-modules-cache-path
+q
+# This test checks that we get *any* clang modules cache path by default. The
+# actual path depends on the operating system.
+# CHECK: symbols.clang-modules-cache-path (file) = "
+# Clang treats an empty path in the same way as 'no path', so explicitly check
+# that we never have an empty path by default.
+# CHECK-NOT: symbols.clang-modules-cache-path (file) = ""
Index: lldb/source/Core/ModuleList.cpp
===
--- lldb/source/Core/ModuleList.cpp
+++ lldb/source/Core/ModuleList.cpp
@@ -82,8 +82,9 @@
[this] { UpdateSymlinkMappings(); 
});
 
   llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  if (clang::driver::Driver::getDefaultModuleCachePath(path)) {
+lldbassert(SetClangModulesCachePath(FileSpec(path)));
+  }
 }
 
 bool ModuleListProperties::GetEnableExternalLookup() const {
@@ -104,8 +105,8 @@
   ->GetCurrentValue();
 }
 
-bool ModuleListProperties::SetClangModulesCachePath(llvm::StringRef path) {
-  return m_collection_sp->SetPropertyAtIndexAsString(
+bool ModuleListProperties::SetClangModulesCachePath(const FileSpec &path) {
+  return m_collection_sp->SetPropertyAtIndexAsFileSpec(
   nullptr, ePropertyClangModulesCachePath, path);
 }
 
Index: lldb/include/lldb/Core/ModuleList.h
===
--- lldb/include/lldb/Core/ModuleList.h
+++ lldb/include/lldb/Core/ModuleList.h
@@ -56,7 +56,7 @@
   ModuleListProperties();
 
   FileSpec GetClangModulesCachePath() const;
-  bool SetClangModulesCachePath(llvm::StringRef path);
+  bool SetClangModulesCachePath(const FileSpec &path);
   bool GetEnableExternalLookup() const;
   bool SetEnableExternalLookup(bool new_value);
 


Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -54,6 +54,10 @@
   command=FindTool('lldb'),
   extra_args=['-S', lldb_init],
   unresolved='fatal'),
+ToolSubst('%lldb-noinit',
+  command=FindTool('lldb'),
+  extra_args=['--no-lldbinit'],
+  unresolved='fatal'),
 ToolSubst('%lldb-server',
   command=FindTool("lldb-server"),
   extra_args=[],
Index: lldb/test/Shell/Settings/TestDefaultModuleCachePath.test
===
--- /dev/null
+++ lldb/test/Shell/Settings/TestDefaultModuleCachePath.test
@@ -0,0 +1,9 @@
+# RUN: %lldb-noinit -x -s %s | FileCheck %s
+settings show symbols.clang-modules-cache-path
+q
+# This test checks that we get *any* clang modules cache path by default. The
+# actual path depends on the operating system.
+# CHECK: symbols.clang-modules-cache-path (file) = "
+# Clang treats an empty path in the same way as 'no path', so explicitly check
+# that we never have an empty path by default.
+# CHECK-NOT: symbols.clang-modules-cache-path (file) = ""
Index: lldb/source/Core/ModuleList.cpp
===
--- lldb/source/Core/ModuleList.cpp
+++ lldb/source/Core/ModuleList.cpp
@@ -82,8 +82,9 @@
[this] { UpdateSymlinkMappings(); });
 
   llvm::Sma

[Lldb-commits] [lldb] b9f0713 - [lldb/Docs] Fix lldb-x86_64-fedora URL as it is still a silent bot

2020-12-10 Thread Jan Kratochvil via lldb-commits

Author: Jan Kratochvil
Date: 2020-12-10T13:52:10+01:00
New Revision: b9f0713f73a5aae97262f3cf07d74a96cbef8aa8

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

LOG: [lldb/Docs] Fix lldb-x86_64-fedora URL as it is still a silent bot

Added: 


Modified: 
lldb/docs/resources/bots.rst

Removed: 




diff  --git a/lldb/docs/resources/bots.rst b/lldb/docs/resources/bots.rst
index 926259bd92be..f80a2333992b 100644
--- a/lldb/docs/resources/bots.rst
+++ b/lldb/docs/resources/bots.rst
@@ -11,7 +11,7 @@ LLVM Buildbot is the place where volunteers provide build 
machines. Everyone can
 * `lldb-x86_64-debian `_
 * `lldb-aarch64-ubuntu `_
 * `lldb-arm-ubuntu `_
-* `lldb-x86_64-fedora `_
+* `lldb-x86_64-fedora `_
 
 An overview of all LLDB builders can be found here:
 



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


[Lldb-commits] [PATCH] D92872: [lldb] [docs] Add a manpage for lldb-server

2020-12-10 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Thanks for writing this.




Comment at: lldb/docs/man/lldb-server.rst:108
+If neither of target options are used, :program:`lldb-server` is started
+without a specific target. It can be afterwards instructedb the client
+to launch or attach.

s/instructedb/instructed


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

https://reviews.llvm.org/D92872

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


[Lldb-commits] [PATCH] D92872: [lldb] [docs] Add a manpage for lldb-server

2020-12-10 Thread Michał Górny via Phabricator via lldb-commits
mgorny marked an inline comment as done.
mgorny added inline comments.



Comment at: lldb/docs/man/lldb-server.rst:108
+If neither of target options are used, :program:`lldb-server` is started
+without a specific target. It can be afterwards instructedb the client
+to launch or attach.

labath wrote:
> s/instructedb/instructed
Actually 'instructed by' ;-).


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

https://reviews.llvm.org/D92872

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


[Lldb-commits] [lldb] 25c40a4 - [lldb] [docs] Add a manpage for lldb-server

2020-12-10 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-12-10T15:02:25+01:00
New Revision: 25c40a45999e59e3b2902cd91373cd47e7a93488

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

LOG: [lldb] [docs] Add a manpage for lldb-server

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

Added: 
lldb/docs/man/lldb-server.rst

Modified: 
lldb/docs/conf.py

Removed: 




diff  --git a/lldb/docs/conf.py b/lldb/docs/conf.py
index cdab1b00e1b7..4d894bf050da 100644
--- a/lldb/docs/conf.py
+++ b/lldb/docs/conf.py
@@ -246,7 +246,9 @@
 
 # One entry per manual page. List of tuples
 # (source start file, name, description, authors, manual section).
-man_pages = [('man/lldb', 'lldb', u'LLDB Documentation', [u'LLVM project'], 1)]
+man_pages = [('man/lldb', 'lldb', u'LLDB Documentation', [u'LLVM project'], 1),
+ ('man/lldb-server', 'lldb-server', u'LLDB Documentation', [u'LLVM 
project'], 1),
+ ]
 
 # If true, show URL addresses after external links.
 #man_show_urls = False

diff  --git a/lldb/docs/man/lldb-server.rst b/lldb/docs/man/lldb-server.rst
new file mode 100644
index ..a67c00b305f6
--- /dev/null
+++ b/lldb/docs/man/lldb-server.rst
@@ -0,0 +1,209 @@
+:orphan:
+
+lldb-server -- Server for LLDB Debugging Sessions
+=
+
+.. program:: lldb-server
+
+SYNOPSIS
+
+
+| :program:`lldb-server` v[ersion]
+| :program:`lldb-server` g[dbserver] [*options*]
+| :program:`lldb-server` p[latform] [*options*]
+
+DESCRIPTION
+---
+
+:program:`lldb-server` provides the server counterpart of the LLVM debugger.
+The server runs and monitors the debugged program, while the user interfaces
+with it via a client, either running locally or connecting remotely.
+
+All of the code in the LLDB project is available under the Apache 2.0 License
+with LLVM exceptions.
+
+COMMANDS
+
+
+The first argument to lldb-server specifies a command to run.
+
+.. option:: v[ersion]
+
+ Prints lldb-server version and exits.
+
+.. option:: g[dbserver]
+
+ Runs the server using the gdb-remote protocol. LLDB can afterwards
+ connect to the server using *gdb-remote* command.
+
+.. option:: p[latform]
+
+ Runs the platform server. LLDB can afterwards connect to the server using
+ *platform select*, followed by *platform connect*.
+
+GDBSERVER COMMAND
+-
+
+| :program:`lldb-server` g[dbserver] [*options*] [[*host*]:*port*] [[--] 
*program* *args*...]
+
+CONNECTION
+~~
+
+.. option:: host:port
+
+ Specifies the hostname and TCP port to listen on. Obligatory unless another
+ listening option is used. If host is empty, *localhost* will be used.  If port
+ is zero, a random port will be selected, and written as specified by --pipe
+ or --named-pipe options.
+
+.. option:: --fd 
+
+ Communicate over the given file descriptor instead of sockets.
+
+.. option:: --named-pipe 
+
+ Write the listening port number to the specified named pipe.
+
+.. option:: --pipe 
+
+ Write the listening port number to the specified pipe (fd).
+
+.. option:: --reverse-connect
+
+ Connect to the client instead of passively waiting for a connection. In this
+ case, [host]:port denotes the remote address to connect to.
+
+GENERAL OPTIONS
+~~~
+
+.. option:: --help
+
+ Prints out the usage information and exits.
+
+.. option:: --log-channels 
+
+ Channels to log. A colon-separated list of entries. Each entry starts with
+ a channel followed by a space-separated list of categories.
+
+.. option:: --log-file 
+
+ Destination file to log to. If empty, log to stderr.
+
+.. option:: --setsid
+
+ Run lldb-server in a new session.
+
+TARGET SELECTION
+
+
+.. option:: --attach 
+
+ Attach to the process given by a (numeric) process id or a name.
+
+.. option:: -- program args
+
+ Launch a program for debugging.
+
+If neither of target options are used, :program:`lldb-server` is started
+without a specific target. It can be afterwards instructed by the client
+to launch or attach.
+
+PLATFORM COMMAND
+
+
+| :program:`lldb-server` p[latform] [*options*] --server --listen 
[[*host*]:*port*]
+
+CONNECTION
+~~
+
+.. option:: --server
+
+ Run in server mode, handling multiple connections. If this is not specified,
+ lldb-server will accept only one connection and exit when it is finished.
+
+.. option:: --listen :
+
+ Hostname and port to listen on. Obligatory. If *port* is zero, a random port
+ will be used.
+
+.. option:: --socket-file 
+
+ Write the listening socket port number to the specified file.
+
+GENERAL OPTIONS
+~~~
+
+.. option:: --log-channels 
+
+ Channels to log. A colon-separated list of entries. Each entry starts with
+ a channel followed by a space-separated list of categories.
+
+.. option:: 

[Lldb-commits] [PATCH] D92872: [lldb] [docs] Add a manpage for lldb-server

2020-12-10 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
mgorny marked an inline comment as done.
Closed by commit rG25c40a45999e: [lldb] [docs] Add a manpage for lldb-server 
(authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D92872?vs=310291&id=310871#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92872

Files:
  lldb/docs/conf.py
  lldb/docs/man/lldb-server.rst

Index: lldb/docs/man/lldb-server.rst
===
--- /dev/null
+++ lldb/docs/man/lldb-server.rst
@@ -0,0 +1,209 @@
+:orphan:
+
+lldb-server -- Server for LLDB Debugging Sessions
+=
+
+.. program:: lldb-server
+
+SYNOPSIS
+
+
+| :program:`lldb-server` v[ersion]
+| :program:`lldb-server` g[dbserver] [*options*]
+| :program:`lldb-server` p[latform] [*options*]
+
+DESCRIPTION
+---
+
+:program:`lldb-server` provides the server counterpart of the LLVM debugger.
+The server runs and monitors the debugged program, while the user interfaces
+with it via a client, either running locally or connecting remotely.
+
+All of the code in the LLDB project is available under the Apache 2.0 License
+with LLVM exceptions.
+
+COMMANDS
+
+
+The first argument to lldb-server specifies a command to run.
+
+.. option:: v[ersion]
+
+ Prints lldb-server version and exits.
+
+.. option:: g[dbserver]
+
+ Runs the server using the gdb-remote protocol. LLDB can afterwards
+ connect to the server using *gdb-remote* command.
+
+.. option:: p[latform]
+
+ Runs the platform server. LLDB can afterwards connect to the server using
+ *platform select*, followed by *platform connect*.
+
+GDBSERVER COMMAND
+-
+
+| :program:`lldb-server` g[dbserver] [*options*] [[*host*]:*port*] [[--] *program* *args*...]
+
+CONNECTION
+~~
+
+.. option:: host:port
+
+ Specifies the hostname and TCP port to listen on. Obligatory unless another
+ listening option is used. If host is empty, *localhost* will be used.  If port
+ is zero, a random port will be selected, and written as specified by --pipe
+ or --named-pipe options.
+
+.. option:: --fd 
+
+ Communicate over the given file descriptor instead of sockets.
+
+.. option:: --named-pipe 
+
+ Write the listening port number to the specified named pipe.
+
+.. option:: --pipe 
+
+ Write the listening port number to the specified pipe (fd).
+
+.. option:: --reverse-connect
+
+ Connect to the client instead of passively waiting for a connection. In this
+ case, [host]:port denotes the remote address to connect to.
+
+GENERAL OPTIONS
+~~~
+
+.. option:: --help
+
+ Prints out the usage information and exits.
+
+.. option:: --log-channels 
+
+ Channels to log. A colon-separated list of entries. Each entry starts with
+ a channel followed by a space-separated list of categories.
+
+.. option:: --log-file 
+
+ Destination file to log to. If empty, log to stderr.
+
+.. option:: --setsid
+
+ Run lldb-server in a new session.
+
+TARGET SELECTION
+
+
+.. option:: --attach 
+
+ Attach to the process given by a (numeric) process id or a name.
+
+.. option:: -- program args
+
+ Launch a program for debugging.
+
+If neither of target options are used, :program:`lldb-server` is started
+without a specific target. It can be afterwards instructed by the client
+to launch or attach.
+
+PLATFORM COMMAND
+
+
+| :program:`lldb-server` p[latform] [*options*] --server --listen [[*host*]:*port*]
+
+CONNECTION
+~~
+
+.. option:: --server
+
+ Run in server mode, handling multiple connections. If this is not specified,
+ lldb-server will accept only one connection and exit when it is finished.
+
+.. option:: --listen :
+
+ Hostname and port to listen on. Obligatory. If *port* is zero, a random port
+ will be used.
+
+.. option:: --socket-file 
+
+ Write the listening socket port number to the specified file.
+
+GENERAL OPTIONS
+~~~
+
+.. option:: --log-channels 
+
+ Channels to log. A colon-separated list of entries. Each entry starts with
+ a channel followed by a space-separated list of categories.
+
+.. option:: --log-file 
+
+ Destination file to log to. If empty, log to stderr.
+
+GDB-SERVER CONNECTIONS
+~~
+
+.. option:: --gdbserver-port 
+
+ Define a port to be used for gdb-server connections. Can be specified multiple
+ times to allow multiple ports. Has no effect if --min-gdbserver-port
+ and --max-gdbserver-port are specified.
+
+.. option:: --min-gdbserver-port 
+.. option:: --max-gdbserver-port 
+
+ Specify the range of ports that can be used for gdb-server connections. Both
+ options need to be specified simultaneously. Overrides --gdbserver-port.
+
+.. option:: --port-offset 
+
+ Add the specified offset to port numbers returned by server. This is useful
+ if the server is runni

[Lldb-commits] [PATCH] D92187: [lldb] [POSIX-DYLD] Add libraries from initial rendezvous brkpt hit

2020-12-10 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 310877.
mgorny added a comment.

Avoid adding duplicate entry for ld-linux.


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

https://reviews.llvm.org/D92187

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
  lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
  lldb/test/API/api/multithreaded/TestMultithreaded.py
  
lldb/test/API/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
  
lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
  lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test

Index: lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test
===
--- lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test
+++ lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test
@@ -3,7 +3,6 @@
 
 # REQUIRES: target-x86_64
 # UNSUPPORTED: system-windows
-# XFAIL: system-freebsd
 
 # RUN: %clang_host %p/Inputs/call-asm.c -x assembler-with-cpp %p/Inputs/thread-step-out-ret-addr-check.s -o %t
 # RUN: not %lldb %t -s %s -b 2>&1 | FileCheck %s
Index: lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
===
--- lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
+++ lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
@@ -120,7 +120,7 @@
 
 @llgs_test
 @skipUnlessPlatform(["linux", "android", "freebsd", "netbsd"])
-@expectedFailureAll(oslist=["freebsd", "netbsd"])
+@expectedFailureNetBSD
 def test_libraries_svr4_load_addr(self):
 self.setup_test()
 self.libraries_svr4_has_correct_load_addr()
Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -843,7 +843,6 @@
 self.qMemoryRegionInfo_is_supported()
 
 @llgs_test
-@expectedFailureAll(oslist=["freebsd"])
 def test_qMemoryRegionInfo_is_supported_llgs(self):
 self.init_llgs_test()
 self.build()
@@ -908,7 +907,6 @@
 self.qMemoryRegionInfo_reports_code_address_as_executable()
 
 @skipIfWindows # No pty support to test any inferior output
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test_qMemoryRegionInfo_reports_code_address_as_executable_llgs(self):
 self.init_llgs_test()
@@ -975,7 +973,6 @@
 self.qMemoryRegionInfo_reports_stack_address_as_readable_writeable()
 
 @skipIfWindows # No pty support to test any inferior output
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test_qMemoryRegionInfo_reports_stack_address_as_readable_writeable_llgs(
 self):
@@ -1042,7 +1039,6 @@
 self.qMemoryRegionInfo_reports_heap_address_as_readable_writeable()
 
 @skipIfWindows # No pty support to test any inferior output
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test_qMemoryRegionInfo_reports_heap_address_as_readable_writeable_llgs(
 self):
Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -23,7 +23,6 @@
 'main.cpp',
 '// Run here before printing memory regions')
 
-@expectedFailureAll(oslist=["freebsd"])
 def test(self):
 self.build()
 
Index: lldb/test/API/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
===
--- lldb/test/API/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
+++ lldb/test/API/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
@@ -15,7 +15,6 @@
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
-@expectedFailureAll(oslist=["freebsd"], bugnumber='llvm.org/pr48373')
 @expectedFailureNetBSD
 def test(self):
 self.build()
Index: lldb/test/API/api/multithreaded/TestMultithreaded.py
===
--- lldb/test/API/api/multithreaded/TestMultithreaded.py
+++ lldb/test/API/api/multithreaded/TestMultithreaded.py
@@ -31,7 +31,6 @@
 @skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
-@expectedFailureAll(oslist=['fre

[Lldb-commits] [lldb] db84208 - [lldb/test] Replace ad-hoc server test choice with test categories

2020-12-10 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-12-10T16:21:28+01:00
New Revision: db8420825073371ddc077b020634e71e315e38a1

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

LOG: [lldb/test] Replace ad-hoc server test choice with test categories

This makes things consistent, and enables further simplifications down
the road.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/test_categories.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index bcf665f6cc4c..ff445fa0b926 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -373,20 +373,12 @@ def should_skip_simulator_test():
 
 def debugserver_test(func):
 """Decorate the item as a debugserver test."""
-def should_skip_debugserver_test():
-return ("debugserver tests"
-if not configuration.debugserver_platform
-else None)
-return skipTestIfFn(should_skip_debugserver_test)(func)
+return add_test_categories(["debugserver"])(func)
 
 
 def llgs_test(func):
 """Decorate the item as a lldb-server test."""
-def should_skip_llgs_tests():
-return ("llgs tests"
-if not configuration.llgs_platform
-else None)
-return skipTestIfFn(should_skip_llgs_tests)(func)
+return add_test_categories(["llgs"])(func)
 
 
 def expectedFailureOS(

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 64a197872f9e..86ea34ee2582 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -850,6 +850,14 @@ def checkDebugInfoSupport():
 if skipped:
 print("Skipping following debug info categories:", skipped)
 
+def checkDebugServerSupport():
+from lldbsuite.test import lldbplatformutil
+
+if lldbplatformutil.platformIsDarwin():
+configuration.skip_categories.append("llgs")
+else:
+configuration.skip_categories.append("debugserver")
+
 def run_suite():
 # On MacOS X, check to make sure that domain for com.apple.DebugSymbols 
defaults
 # does not exist before proceeding to running the test suite.
@@ -944,15 +952,9 @@ def run_suite():
 checkLibstdcxxSupport()
 checkWatchpointSupport()
 checkDebugInfoSupport()
+checkDebugServerSupport()
 checkObjcSupport()
 
-# Perform LLGS tests only on platforms using it.
-configuration.llgs_platform = (
-target_platform in ["freebsd", "linux", "netbsd", "windows"])
-
-# Perform debugserver tests elsewhere (i.e. on Darwin platforms).
-configuration.debugserver_platform = not configuration.llgs_platform
-
 for testdir in configuration.testdirs:
 for (dirpath, dirnames, filenames) in os.walk(testdir):
 visit('Test', dirpath, filenames)

diff  --git a/lldb/packages/Python/lldbsuite/test/test_categories.py 
b/lldb/packages/Python/lldbsuite/test/test_categories.py
index 699fcf4cb887..9f1196ea6497 100644
--- a/lldb/packages/Python/lldbsuite/test/test_categories.py
+++ b/lldb/packages/Python/lldbsuite/test/test_categories.py
@@ -23,6 +23,7 @@
 'cmdline': 'Tests related to the LLDB command-line interface',
 'darwin-log': 'Darwin log tests',
 'dataformatters': 'Tests related to the type command and the data 
formatters subsystem',
+'debugserver': 'Debugserver tests',
 'dsym': 'Tests that can be run with DSYM debug information',
 'dwarf': 'Tests that can be run with DWARF debug information',
 'dwo': 'Tests that can be run with DWO debug information',
@@ -35,6 +36,7 @@
 'libstdcxx': 'Test for libstdcxx data formatters',
 'lldb-server': 'Tests related to lldb-server',
 'lldb-vscode': 'Visual Studio Code debug adaptor tests',
+'llgs': 'Tests for the gdb-server functionality of lldb-server',
 'objc': 'Tests related to the Objective-C programming language support',
 'pyapi': 'Tests related to the Python API',
 'std-module': 'Tests related to importing the std module',



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


[Lldb-commits] [lldb] b505142 - [lldb/test] Change base class of lldb-server tests

2020-12-10 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-12-10T16:21:28+01:00
New Revision: b505142fa5d301238796318d2d092d6fb3bd2d31

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

LOG: [lldb/test] Change base class of lldb-server tests

lldb-server tests are a very special subclass of "api" tests. As they
communicate with lldb-server directly, they don't actually need most of
facilities provided by our TestBase class. In particular, they don't
need the ability to fork debug info flavours of tests (but they could
use debug server flavours).

This makes them inherit from "Base" instead. This avoids the need to
explicitly mark these tests as NO_DEBUG_INFO_TEST_CASE. Two additional
necessary tweaks were:
- move run_platform_command to the base (Base) class. This is used in
  one test, and can be generally useful when running tests remotely.
- add a "build" method, forwarding to buildDefault. This is to avoid
  updating each test case to use buildDefault (also, "build" sounds
  better). It might be interesting to refactor the (Test)Base classes so
  that all debug info flavour handling happens in TestBase, and the Base
  class provides a simple build method automatically.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 1f3bc7722290..d240050cc4c6 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1785,6 +1785,12 @@ def getLibcPlusPlusLibs(self):
 else:
 return ['libc++.1.dylib', 'libc++abi.']
 
+def run_platform_command(self, cmd):
+platform = self.dbg.GetSelectedPlatform()
+shell_command = lldb.SBPlatformShellCommand(cmd)
+err = platform.Run(shell_command)
+return (err, shell_command.GetStatus(), shell_command.GetOutput())
+
 # Metaclass for TestBase to change the list of test metods when a new TestCase 
is loaded.
 # We change the test methods to create a new test method for each test for 
each debug info we are
 # testing. The name of the new test method will be 
'_' and with adding
@@ -2656,12 +2662,6 @@ def build(
 else:
 self.fail("Can't build for debug info: %s" % self.getDebugInfo())
 
-def run_platform_command(self, cmd):
-platform = self.dbg.GetSelectedPlatform()
-shell_command = lldb.SBPlatformShellCommand(cmd)
-err = platform.Run(shell_command)
-return (err, shell_command.GetStatus(), shell_command.GetOutput())
-
 """Assert that an lldb.SBError is in the "success" state."""
 def assertSuccess(self, obj, msg=None):
 if not obj.Success():

diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
index 2908ca2809a9..b578aae12062 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -27,9 +27,7 @@ class _ConnectionRefused(IOError):
 pass
 
 
-class GdbRemoteTestCaseBase(TestBase):
-
-NO_DEBUG_INFO_TESTCASE = True
+class GdbRemoteTestCaseBase(Base):
 
 # Default time out in seconds. The timeout is increased tenfold under Asan.
 DEFAULT_TIMEOUT =  20 * (10 if ('ASAN_OPTIONS' in os.environ) else 1)
@@ -83,7 +81,7 @@ def isVerboseLoggingRequested(self):
for channel in lldbtest_config.channels)
 
 def setUp(self):
-TestBase.setUp(self)
+super(GdbRemoteTestCaseBase, self).setUp()
 
 self.setUpBaseLogging()
 self.debug_monitor_extra_args = []
@@ -121,6 +119,9 @@ def tearDown(self):
 self._verbose_log_handler = None
 TestBase.tearDown(self)
 
+def build(self, *args, **kwargs):
+self.buildDefault(*args, **kwargs)
+
 def getLocalServerLogFile(self):
 return self.log_basename + "-server.log"
 



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


[Lldb-commits] [lldb] 839e845 - [lldb] Remove assumption from Clang-based data formatters that their types are in the scratch AST

2020-12-10 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-12-10T17:35:03+01:00
New Revision: 839e845277894ad37fbca8063cbf1955331fbeff

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

LOG: [lldb] Remove assumption from Clang-based data formatters that their types 
are in the scratch AST

Several data formatters assume their types are in the Target's scratch AST and
build new types from that scratch AST instance. However, types from different
ASTs shouldn't be mixed, so this (unchecked) assumption may lead to problems if
we ever have more than one scratch AST or someone somehow invokes data
formatters on a type that are not in the scratch AST.

Instead we can use in all the formatters just the TypeSystem of the type we're
formatting. That's much simpler and avoids all the headache of finding the right
TypeSystem that matches the one of the formatted type.

Right now LLDB only has one scratch TypeSystemClang instance and we format only
types that are in the scratch AST, so this doesn't change anything in the
current way LLDB works. The intention here is to allow follow up refactorings
that introduce multiple scratch ASTs with the same Target.

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

Added: 


Modified: 
lldb/source/DataFormatters/VectorType.cpp
lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
lldb/source/Plugins/Language/ObjC/CoreMedia.cpp

Removed: 




diff  --git a/lldb/source/DataFormatters/VectorType.cpp 
b/lldb/source/DataFormatters/VectorType.cpp
index fd1c0bc96cd4..cc24bb1de428 100644
--- a/lldb/source/DataFormatters/VectorType.cpp
+++ b/lldb/source/DataFormatters/VectorType.cpp
@@ -220,20 +220,8 @@ class VectorTypeSyntheticFrontEnd : public 
SyntheticChildrenFrontEnd {
 CompilerType parent_type(m_backend.GetCompilerType());
 CompilerType element_type;
 parent_type.IsVectorType(&element_type, nullptr);
-TypeSystem *type_system = nullptr;
-if (auto target_sp = m_backend.GetTargetSP()) {
-  auto type_system_or_err =
-  target_sp->GetScratchTypeSystemForLanguage(lldb::eLanguageTypeC);
-  if (auto err = type_system_or_err.takeError()) {
-LLDB_LOG_ERROR(
-lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DATAFORMATTERS),
-std::move(err), "Unable to update from scratch TypeSystem");
-  } else {
-type_system = &type_system_or_err.get();
-  }
-}
-m_child_type =
-::GetCompilerTypeForFormat(m_parent_format, element_type, type_system);
+m_child_type = ::GetCompilerTypeForFormat(m_parent_format, element_type,
+  parent_type.GetTypeSystem());
 m_num_children = ::CalculateNumChildren(parent_type, m_child_type);
 m_item_format = GetItemFormatForFormat(m_parent_format, m_child_type);
 return false;

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
index 42f6bd9ffb7b..35788a6445c2 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
@@ -50,11 +50,7 @@ class BlockPointerSyntheticFrontEnd : public 
SyntheticChildrenFrontEnd {
 }
 
 TypeSystemClang *clang_ast_context =
-llvm::dyn_cast(&type_system_or_err.get());
-
-if (!clang_ast_context) {
-  return;
-}
+llvm::cast(block_pointer_type.GetTypeSystem());
 
 std::shared_ptr clang_ast_importer;
 auto *state = target_sp->GetPersistentExpressionStateForLanguage(

diff  --git a/lldb/source/Plugins/Language/ObjC/CoreMedia.cpp 
b/lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
index ac2f45b8354f..efc80cc75557 100644
--- a/lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
+++ b/lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
@@ -25,21 +25,12 @@ bool lldb_private::formatters::CMTimeSummaryProvider(
   if (!type.IsValid())
 return false;
 
-  auto type_system_or_err =
-  valobj.GetExecutionContextRef()
-  .GetTargetSP()
-  ->GetScratchTypeSystemForLanguage(lldb::eLanguageTypeC);
-  if (auto err = type_system_or_err.takeError()) {
-LLDB_LOG_ERROR(
-lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DATAFORMATTERS),
-std::move(err), "Failed to get scratch type system");
-return false;
-  }
+  TypeSystem *type_system = type.GetTypeSystem();
   // fetch children by offset to compensate for potential lack of debug info
-  auto int64_ty = type_system_or_err->GetBuiltinTypeForEncodingAndBitSize(
-  eEncodingSint, 64);
-  auto int32_ty = type_system_or_err->GetBuiltinTypeForEncodingAndBitSize(
-  eEncodingSint, 32);
+  auto int64_ty =
+  type_system->GetBuiltinTypeForEncodingAndBitSize(eEncodingSint, 64);
+  auto int32_ty =
+  type_syst

[Lldb-commits] [PATCH] D92757: [lldb] Remove assumption from Clang-based data formatters that their types are in the scratch AST

2020-12-10 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG839e84527789: [lldb] Remove assumption from Clang-based data 
formatters that their types are… (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92757

Files:
  lldb/source/DataFormatters/VectorType.cpp
  lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
  lldb/source/Plugins/Language/ObjC/CoreMedia.cpp


Index: lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
===
--- lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
+++ lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
@@ -25,21 +25,12 @@
   if (!type.IsValid())
 return false;
 
-  auto type_system_or_err =
-  valobj.GetExecutionContextRef()
-  .GetTargetSP()
-  ->GetScratchTypeSystemForLanguage(lldb::eLanguageTypeC);
-  if (auto err = type_system_or_err.takeError()) {
-LLDB_LOG_ERROR(
-lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DATAFORMATTERS),
-std::move(err), "Failed to get scratch type system");
-return false;
-  }
+  TypeSystem *type_system = type.GetTypeSystem();
   // fetch children by offset to compensate for potential lack of debug info
-  auto int64_ty = type_system_or_err->GetBuiltinTypeForEncodingAndBitSize(
-  eEncodingSint, 64);
-  auto int32_ty = type_system_or_err->GetBuiltinTypeForEncodingAndBitSize(
-  eEncodingSint, 32);
+  auto int64_ty =
+  type_system->GetBuiltinTypeForEncodingAndBitSize(eEncodingSint, 64);
+  auto int32_ty =
+  type_system->GetBuiltinTypeForEncodingAndBitSize(eEncodingSint, 32);
 
   auto value_sp(valobj.GetSyntheticChildAtOffset(0, int64_ty, true));
   auto timescale_sp(valobj.GetSyntheticChildAtOffset(8, int32_ty, true));
Index: lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
@@ -50,11 +50,7 @@
 }
 
 TypeSystemClang *clang_ast_context =
-llvm::dyn_cast(&type_system_or_err.get());
-
-if (!clang_ast_context) {
-  return;
-}
+llvm::cast(block_pointer_type.GetTypeSystem());
 
 std::shared_ptr clang_ast_importer;
 auto *state = target_sp->GetPersistentExpressionStateForLanguage(
Index: lldb/source/DataFormatters/VectorType.cpp
===
--- lldb/source/DataFormatters/VectorType.cpp
+++ lldb/source/DataFormatters/VectorType.cpp
@@ -220,20 +220,8 @@
 CompilerType parent_type(m_backend.GetCompilerType());
 CompilerType element_type;
 parent_type.IsVectorType(&element_type, nullptr);
-TypeSystem *type_system = nullptr;
-if (auto target_sp = m_backend.GetTargetSP()) {
-  auto type_system_or_err =
-  target_sp->GetScratchTypeSystemForLanguage(lldb::eLanguageTypeC);
-  if (auto err = type_system_or_err.takeError()) {
-LLDB_LOG_ERROR(
-lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DATAFORMATTERS),
-std::move(err), "Unable to update from scratch TypeSystem");
-  } else {
-type_system = &type_system_or_err.get();
-  }
-}
-m_child_type =
-::GetCompilerTypeForFormat(m_parent_format, element_type, type_system);
+m_child_type = ::GetCompilerTypeForFormat(m_parent_format, element_type,
+  parent_type.GetTypeSystem());
 m_num_children = ::CalculateNumChildren(parent_type, m_child_type);
 m_item_format = GetItemFormatForFormat(m_parent_format, m_child_type);
 return false;


Index: lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
===
--- lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
+++ lldb/source/Plugins/Language/ObjC/CoreMedia.cpp
@@ -25,21 +25,12 @@
   if (!type.IsValid())
 return false;
 
-  auto type_system_or_err =
-  valobj.GetExecutionContextRef()
-  .GetTargetSP()
-  ->GetScratchTypeSystemForLanguage(lldb::eLanguageTypeC);
-  if (auto err = type_system_or_err.takeError()) {
-LLDB_LOG_ERROR(
-lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DATAFORMATTERS),
-std::move(err), "Failed to get scratch type system");
-return false;
-  }
+  TypeSystem *type_system = type.GetTypeSystem();
   // fetch children by offset to compensate for potential lack of debug info
-  auto int64_ty = type_system_or_err->GetBuiltinTypeForEncodingAndBitSize(
-  eEncodingSint, 64);
-  auto int32_ty = type_system_or_err->GetBuiltinTypeForEncodingAndBitSize(
-  eEncodingSint, 32);
+  auto int64_ty =

[Lldb-commits] [PATCH] D92820: [lldb] Deal gracefully with concurrency in the API instrumentation.

2020-12-10 Thread Frederic Riss via Phabricator via lldb-commits
friss accepted this revision.
friss added a comment.
This revision is now accepted and ready to land.

This LGTM.
There is one thing that you might want to address, but I'll leave it up to you 
(and even if you do it can be a different commit): with the introduction of 
`idx` variables, it becomes easy to get confused between `id` and `idx`(Or 
`CheckID` and `CheckIndex`) in the codebase. Not sure how to disambiguate, or 
if it's really worth it.


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

https://reviews.llvm.org/D92820

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


[Lldb-commits] [PATCH] D92820: [lldb] Deal gracefully with concurrency in the API instrumentation.

2020-12-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

In D92820#2446091 , @friss wrote:

> This LGTM.
> There is one thing that you might want to address, but I'll leave it up to 
> you (and even if you do it can be a different commit): with the introduction 
> of `idx` variables, it becomes easy to get confused between `id` and `idx`(Or 
> `CheckID` and `CheckIndex`) in the codebase. Not sure how to disambiguate, or 
> if it's really worth it.

Agreed, it's even more confusing because I also use `idx` for the object index. 
Let me rename this to sequence.


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

https://reviews.llvm.org/D92820

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


[Lldb-commits] [PATCH] D92164: Make CommandInterpreter's execution context the same as debugger's one.

2020-12-10 Thread Tatyana Krasnukha via Phabricator via lldb-commits
tatyana-krasnukha updated this revision to Diff 310927.
tatyana-krasnukha retitled this revision from "Make SBDebugger internal 
variable getter and setter not use CommandInterpreter's context" to "Make 
CommandInterpreter's execution context the same as debugger's one.".
tatyana-krasnukha edited the summary of this revision.
tatyana-krasnukha added a comment.

Addressed comments


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

https://reviews.llvm.org/D92164

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/source/Breakpoint/BreakpointOptions.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectRegexCommand.cpp
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/python_api/debugger/Makefile
  lldb/test/API/python_api/debugger/TestDebuggerAPI.py
  lldb/test/API/python_api/debugger/main.cpp

Index: lldb/test/API/python_api/debugger/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/debugger/main.cpp
@@ -0,0 +1,9 @@
+// This simple program is to test the lldb Python API SBDebugger.
+
+int func(int val) {
+return val - 1;
+}
+
+int main (int argc, char const *argv[]) {
+return func(argc);
+}
Index: lldb/test/API/python_api/debugger/TestDebuggerAPI.py
===
--- lldb/test/API/python_api/debugger/TestDebuggerAPI.py
+++ lldb/test/API/python_api/debugger/TestDebuggerAPI.py
@@ -43,3 +43,54 @@
 target = lldb.SBTarget()
 self.assertFalse(target.IsValid())
 self.dbg.DeleteTarget(target)
+
+def test_debugger_internal_variable(self):
+"""Ensure that SBDebugger reachs the same instance of properties
+   regardless CommandInterpreter's context initialization"""
+self.build()
+exe = self.getBuildArtifact("a.out")
+
+# Create a target by the debugger.
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+property_name = "target.process.memory-cache-line-size"
+
+def get_cache_line_size():
+value_list = lldb.SBStringList()
+value_list = self.dbg.GetInternalVariableValue(property_name,
+   self.dbg.GetInstanceName())
+
+self.assertEqual(value_list.GetSize(), 1)
+try:
+return int(value_list.GetStringAtIndex(0))
+except ValueError as error:
+self.fail("Value is not a number: " + error)
+
+# Get global property value while there are no processes.
+global_cache_line_size = get_cache_line_size()
+
+# Run a process via SB interface. CommandInterpreter's execution context
+# remains empty.
+error = lldb.SBError()
+launch_info = lldb.SBLaunchInfo(None)
+launch_info.SetLaunchFlags(lldb.eLaunchFlagStopAtEntry)
+process = target.Launch(launch_info, error)
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# This should change the value of a process's local property.
+new_cache_line_size = global_cache_line_size + 512
+error = self.dbg.SetInternalVariable(property_name,
+ str(new_cache_line_size),
+ self.dbg.GetInstanceName())
+self.assertTrue(error.Success(),
+property_name + " value was changed successfully")
+
+# Check that it was set actually.
+self.assertEqual(get_cache_line_size(), new_cache_line_size)
+
+# Run any command to initialize CommandInterpreter's execution context.
+self.runCmd("target list")
+
+# Test the local property again, is it set to new_cache_line_size?
+self.assertEqual(get_cache_line_size(), new_cache_line_size)
Index: lldb/test/API/python_api/debugger/Makefile
===
--- /dev/null
+++ lldb/test/API/python_api/debugger/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3351,7 +3351,7 @@
   // Force Async:
   bool old_async = debugger.GetAsyncExecution();
   debugger.SetAsyncExecution(true);
-  debugger.GetCommandInterpreter().HandleCommands(GetCommands(), &exc_ctx,
+  debugger.GetCommandInterpreter().HandleCommands(GetCommands(), exc_ctx,
   opti

[Lldb-commits] [lldb] ac25e86 - [lldb] Deal gracefully with concurrency in the API instrumentation.

2020-12-10 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-12-10T09:37:49-08:00
New Revision: ac25e8628c443cddd841c6c91d1c9e23e88969e5

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

LOG: [lldb] Deal gracefully with concurrency in the API instrumentation.

Prevent lldb from crashing when multiple threads are concurrently
accessing the SB API with reproducer capture enabled.

The API instrumentation records both the input arguments and the return
value, but it cannot block for the duration of the API call. Therefore
we introduce a sequence number that allows to to correlate the function
with its result and add locking to ensure those two parts are emitted
atomically.

Using the sequence number, we can detect situations where the return
value does not succeed the function call, in which case we print an
error saying that concurrency is not (currently) supported. In the
future we might attempt to be smarter and read ahead until we've found
the return value matching the current call.

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

Added: 


Modified: 
lldb/include/lldb/Utility/ReproducerInstrumentation.h
lldb/source/Utility/ReproducerInstrumentation.cpp
lldb/unittests/Utility/ReproducerInstrumentationTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/ReproducerInstrumentation.h 
b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
index e4c31522c4fc..c8a98adf85c7 100644
--- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -333,6 +333,7 @@ class Deserializer {
   }
 
   template  const T &HandleReplayResult(const T &t) {
+CheckSequence(Deserialize());
 unsigned result = Deserialize();
 if (is_trivially_serializable::value)
   return t;
@@ -342,6 +343,7 @@ class Deserializer {
 
   /// Store the returned value in the index-to-object mapping.
   template  T &HandleReplayResult(T &t) {
+CheckSequence(Deserialize());
 unsigned result = Deserialize();
 if (is_trivially_serializable::value)
   return t;
@@ -351,6 +353,7 @@ class Deserializer {
 
   /// Store the returned value in the index-to-object mapping.
   template  T *HandleReplayResult(T *t) {
+CheckSequence(Deserialize());
 unsigned result = Deserialize();
 if (is_trivially_serializable::value)
   return t;
@@ -360,6 +363,7 @@ class Deserializer {
   /// All returned types are recorded, even when the function returns a void.
   /// The latter requires special handling.
   void HandleReplayResultVoid() {
+CheckSequence(Deserialize());
 unsigned result = Deserialize();
 assert(result == 0);
 (void)result;
@@ -369,6 +373,10 @@ class Deserializer {
 return m_index_to_object.GetAllObjects();
   }
 
+  void SetExpectedSequence(unsigned sequence) {
+m_expected_sequence = sequence;
+  }
+
 private:
   template  T Read(ValueTag) {
 assert(HasData(sizeof(T)));
@@ -410,11 +418,17 @@ class Deserializer {
 return *(new UnderlyingT(Deserialize()));
   }
 
+  /// Verify that the given sequence number matches what we expect.
+  void CheckSequence(unsigned sequence);
+
   /// Mapping of indices to objects.
   IndexToObject m_index_to_object;
 
   /// Buffer containing the serialized data.
   llvm::StringRef m_buffer;
+
+  /// The result's expected sequence number.
+  llvm::Optional m_expected_sequence;
 };
 
 /// Partial specialization for C-style strings. We read the string value
@@ -745,12 +759,15 @@ class Recorder {
 if (!ShouldCapture())
   return;
 
+std::lock_guard lock(g_mutex);
+unsigned sequence = GetSequenceNumber();
 unsigned id = registry.GetID(uintptr_t(f));
 
 #ifdef LLDB_REPRO_INSTR_TRACE
 Log(id);
 #endif
 
+serializer.SerializeAll(sequence);
 serializer.SerializeAll(id);
 serializer.SerializeAll(args...);
 
@@ -758,6 +775,7 @@ class Recorder {
 typename std::remove_reference::type>::type>::value) {
   m_result_recorded = false;
 } else {
+  serializer.SerializeAll(sequence);
   serializer.SerializeAll(0);
   m_result_recorded = true;
 }
@@ -771,16 +789,20 @@ class Recorder {
 if (!ShouldCapture())
   return;
 
+std::lock_guard lock(g_mutex);
+unsigned sequence = GetSequenceNumber();
 unsigned id = registry.GetID(uintptr_t(f));
 
 #ifdef LLDB_REPRO_INSTR_TRACE
 Log(id);
 #endif
 
+serializer.SerializeAll(sequence);
 serializer.SerializeAll(id);
 serializer.SerializeAll(args...);
 
 // Record result.
+serializer.SerializeAll(sequence);
 serializer.SerializeAll(0);
 m_result_recorded = true;
   }
@@ -806,7 +828,9 @@ class Recorder {
 if (update_boundary)
   UpdateBoundary();
 if (m_serializer && ShouldCapture()) {
+  std::lock_guard loc

[Lldb-commits] [PATCH] D92820: [lldb] Deal gracefully with concurrency in the API instrumentation.

2020-12-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGac25e8628c44: [lldb] Deal gracefully with concurrency in the 
API instrumentation. (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D92820?vs=310649&id=310941#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92820

Files:
  lldb/include/lldb/Utility/ReproducerInstrumentation.h
  lldb/source/Utility/ReproducerInstrumentation.cpp
  lldb/unittests/Utility/ReproducerInstrumentationTest.cpp

Index: lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
===
--- lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
+++ lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
@@ -576,8 +576,11 @@
   std::string str;
   llvm::raw_string_ostream os(str);
 
+  unsigned sequence = 123;
+
   Serializer serializer(os);
-  serializer.SerializeAll(static_cast(1), static_cast(2));
+  serializer.SerializeAll(sequence, static_cast(1));
+  serializer.SerializeAll(sequence, static_cast(2));
   serializer.SerializeAll(&foo, &bar);
 
   llvm::StringRef buffer(os.str());
@@ -597,8 +600,11 @@
   std::string str;
   llvm::raw_string_ostream os(str);
 
+  unsigned sequence = 123;
+
   Serializer serializer(os);
-  serializer.SerializeAll(static_cast(1), static_cast(2));
+  serializer.SerializeAll(sequence, static_cast(1));
+  serializer.SerializeAll(sequence, static_cast(2));
   serializer.SerializeAll(foo, bar);
 
   llvm::StringRef buffer(os.str());
@@ -1114,3 +1120,48 @@
 bar.Validate();
   }
 }
+
+TEST(RecordReplayTest, ValidSequence) {
+  std::string str;
+  llvm::raw_string_ostream os(str);
+
+  {
+auto data = TestInstrumentationDataRAII::GetRecordingData(os);
+
+unsigned sequence = 1;
+int (*f)() = &lldb_private::repro::invoke::method<
+InstrumentedFoo::F>::record;
+unsigned id = g_registry->GetID(uintptr_t(f));
+g_serializer->SerializeAll(sequence, id);
+
+unsigned result = 0;
+g_serializer->SerializeAll(sequence, result);
+  }
+
+  TestingRegistry registry;
+  Deserializer deserializer(os.str());
+  registry.Replay(deserializer);
+}
+
+TEST(RecordReplayTest, InvalidSequence) {
+  std::string str;
+  llvm::raw_string_ostream os(str);
+
+  {
+auto data = TestInstrumentationDataRAII::GetRecordingData(os);
+
+unsigned sequence = 1;
+int (*f)() = &lldb_private::repro::invoke::method<
+InstrumentedFoo::F>::record;
+unsigned id = g_registry->GetID(uintptr_t(f));
+g_serializer->SerializeAll(sequence, id);
+
+unsigned result = 0;
+unsigned invalid_sequence = 2;
+g_serializer->SerializeAll(invalid_sequence, result);
+  }
+
+  TestingRegistry registry;
+  Deserializer deserializer(os.str());
+  EXPECT_DEATH(registry.Replay(deserializer), "");
+}
Index: lldb/source/Utility/ReproducerInstrumentation.cpp
===
--- lldb/source/Utility/ReproducerInstrumentation.cpp
+++ lldb/source/Utility/ReproducerInstrumentation.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Utility/ReproducerInstrumentation.h"
 #include "lldb/Utility/Reproducer.h"
+#include 
 #include 
 #include 
 #include 
@@ -84,6 +85,16 @@
   return r;
 }
 
+void Deserializer::CheckSequence(unsigned sequence) {
+  if (m_expected_sequence && *m_expected_sequence != sequence)
+llvm::report_fatal_error(
+"The result does not match the preceding "
+"function. This is probably the result of concurrent "
+"use of the SB API during capture, which is currently not "
+"supported.");
+  m_expected_sequence.reset();
+}
+
 bool Registry::Replay(const FileSpec &file) {
   auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath());
   if (auto err = error_or_file.getError())
@@ -107,6 +118,7 @@
   setvbuf(stdout, nullptr, _IONBF, 0);
 
   while (deserializer.HasData(1)) {
+unsigned sequence = deserializer.Deserialize();
 unsigned id = deserializer.Deserialize();
 
 #ifndef LLDB_REPRO_INSTR_TRACE
@@ -115,6 +127,7 @@
 llvm::errs() << "Replaying " << id << ": " << GetSignature(id) << "\n";
 #endif
 
+deserializer.SetExpectedSequence(sequence);
 GetReplayer(id)->operator()(deserializer);
   }
 
@@ -181,21 +194,24 @@
 
 Recorder::Recorder()
 : m_serializer(nullptr), m_pretty_func(), m_pretty_args(),
-  m_local_boundary(false), m_result_recorded(true) {
+  m_local_boundary(false), m_result_recorded(true),
+  m_sequence(std::numeric_limits::max()) {
   if (!g_global_boundary) {
 g_global_boundary = true;
 m_local_boundary = true;
+m_sequence = GetNextSequenceNumber();
   }
 }
 
 Recorder::Recorder(llvm::StringRef pretty_func, std::string &&pretty_args)
 : m_serializer(nullptr), m_pretty_func(pretty_func),
   m_pretty_args(pretty_args), m_local_boundary(false),
-  m_res

[Lldb-commits] [PATCH] D93052: "target create" shouldn't save target if the command failed

2020-12-10 Thread Tatyana Krasnukha via Phabricator via lldb-commits
tatyana-krasnukha created this revision.
tatyana-krasnukha added reviewers: clayborg, JDevlieghere, jingham, teemperor.
tatyana-krasnukha added a project: LLDB.
Herald added a subscriber: emaste.
tatyana-krasnukha requested review of this revision.
Herald added a subscriber: lldb-commits.

TargetList::CreateTarget automatically adds created target to the list, 
however, CommandObjectTargetCreate does some additional preparation after 
creating a target and which can fail.
The command should remove the created target if it failed. Since the function 
has many ways to return, scope guard does this safer.

Changes to the TargetList make target adding and selection more transparent by 
splitting CreateTarget into two parts: creation and adding to the list.

Other changes remove unnecessary SetSelectedTarget calls after CreateTarget.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93052

Files:
  lldb/include/lldb/Target/TargetList.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/TargetList.cpp
  lldb/source/Target/TraceSessionFileParser.cpp
  lldb/unittests/Process/ProcessEventDataTest.cpp
  lldb/unittests/Thread/ThreadTest.cpp

Index: lldb/unittests/Thread/ThreadTest.cpp
===
--- lldb/unittests/Thread/ThreadTest.cpp
+++ lldb/unittests/Thread/ThreadTest.cpp
@@ -83,16 +83,11 @@
 } // namespace
 
 TargetSP CreateTarget(DebuggerSP &debugger_sp, ArchSpec &arch) {
-  Status error;
   PlatformSP platform_sp;
   TargetSP target_sp;
-  error = debugger_sp->GetTargetList().CreateTarget(
+  debugger_sp->GetTargetList().CreateTarget(
   *debugger_sp, "", arch, eLoadDependentsNo, platform_sp, target_sp);
 
-  if (target_sp) {
-debugger_sp->GetTargetList().SetSelectedTarget(target_sp.get());
-  }
-
   return target_sp;
 }
 
Index: lldb/unittests/Process/ProcessEventDataTest.cpp
===
--- lldb/unittests/Process/ProcessEventDataTest.cpp
+++ lldb/unittests/Process/ProcessEventDataTest.cpp
@@ -111,16 +111,11 @@
 typedef std::shared_ptr EventSP;
 
 TargetSP CreateTarget(DebuggerSP &debugger_sp, ArchSpec &arch) {
-  Status error;
   PlatformSP platform_sp;
   TargetSP target_sp;
-  error = debugger_sp->GetTargetList().CreateTarget(
+  debugger_sp->GetTargetList().CreateTarget(
   *debugger_sp, "", arch, eLoadDependentsNo, platform_sp, target_sp);
 
-  if (target_sp) {
-debugger_sp->GetTargetList().SetSelectedTarget(target_sp.get());
-  }
-
   return target_sp;
 }
 
Index: lldb/source/Target/TraceSessionFileParser.cpp
===
--- lldb/source/Target/TraceSessionFileParser.cpp
+++ lldb/source/Target/TraceSessionFileParser.cpp
@@ -123,8 +123,6 @@
   ParsedProcess parsed_process;
   parsed_process.target_sp = target_sp;
 
-  m_debugger.GetTargetList().SetSelectedTarget(target_sp.get());
-
   ProcessSP process_sp = target_sp->CreateProcess(
   /*listener*/ nullptr, "trace",
   /*crash_file*/ nullptr,
Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -48,9 +48,12 @@
 LoadDependentFiles load_dependent_files,
 const OptionGroupPlatform *platform_options,
 TargetSP &target_sp) {
-  return CreateTargetInternal(debugger, user_exe_path, triple_str,
-  load_dependent_files, platform_options,
-  target_sp);
+  auto result = TargetList::CreateTargetInternal(debugger, user_exe_path,
+  triple_str, load_dependent_files, platform_options, target_sp);
+
+  if (target_sp && result.Success())
+AddTargetInternal(target_sp, /*do_select*/ true);
+  return result;
 }
 
 Status TargetList::CreateTarget(Debugger &debugger,
@@ -58,8 +61,12 @@
 const ArchSpec &specified_arch,
 LoadDependentFiles load_dependent_files,
 PlatformSP &platform_sp, TargetSP &target_sp) {
-  return CreateTargetInternal(debugger, user_exe_path, specified_arch,
-  load_dependent_files, platform_sp, target_sp);
+  auto result = TargetList::CreateTargetInternal(debugger, user_exe_path,
+  specified_arch, load_dependent_files, platform_sp, target_sp);
+
+  if (target_sp && result.Success())
+AddTargetInternal(target_sp, /*do_select*/ true);
+  return result;
 }
 
 Status TargetList::CreateTargetInternal(
@@ -388,9 +395,6 @@
   

[Lldb-commits] [lldb] 47e7ecd - [lldb] Introduce separate scratch ASTs for debug info types and types imported from C++ modules.

2020-12-10 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-12-10T19:28:01+01:00
New Revision: 47e7ecdd7d36ca0924aa89c0fb2d956a6345a8f5

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

LOG: [lldb] Introduce separate scratch ASTs for debug info types and types 
imported from C++ modules.

Right now we have one large AST for all types in LLDB. All ODR violations in
types we reconstruct are resolved by just letting the ASTImporter handle the
conflicts (either by merging types or somehow trying to introduce a duplicated
declaration in the AST). This works ok for the normal types we build from debug
information as most of them are just simple CXXRecordDecls or empty template
declarations.

However, with a loaded `std` C++ module we have alternative versions of pretty
much all declarations in the `std` namespace that are much more fleshed out than
the debug information declarations. They have all the information that is lost
when converting to DWARF, such as default arguments, template default arguments,
the actual uninstantiated template declarations and so on.

When we merge these C++ module types into the big scratch AST (that might
already contain debug information types) we give the ASTImporter the tricky task
of somehow creating a consistent AST out of all these declarations. Usually this
ends in a messy AST that contains a mostly broken mix of both module and debug
info declarations. The ASTImporter in LLDB is also importing types with the
MinimalImport setting, which usually means the only information we have when
merging two types is often just the name of the declaration and the information
that it contains some child declarations. This makes it pretty much impossible
to even implement a better merging logic (as the names of C++ module
declarations and debug info declarations are identical).

This patch works around this whole merging problem by separating C++ module
types from debug information types. This is done by splitting up the single
scratch AST into two: One default AST for debug information and a dedicated AST
for C++ module types.

The C++ module AST is implemented as a 'specialised AST' that lives within the
default ScratchTypeSystemClang. When we select the scratch AST we can explicitly
request that we want such a isolated sub-AST of the scratch AST. I kept the
infrastructure more general as we probably can use the same mechanism for other
features that introduce conflicting types (such as programs that are compiled
with a custom -wchar-size= option).

There are just two places where we explicitly have request the C++ module AST:
When we export persistent declarations (`$mytype`) and when we create our
persistent result variable (`$0`, `$1`, ...). There are a few formatters that
were previously assuming that there is only one scratch AST which I cleaned up
in a preparation revision here (D92757).

Reviewed By: aprantl

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

Added: 

lldb/test/API/commands/expression/import-std-module/non-module-type-separation/Makefile

lldb/test/API/commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py

lldb/test/API/commands/expression/import-std-module/non-module-type-separation/main.cpp

Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
lldb/unittests/Symbol/TestTypeSystemClang.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
index 66a87ba924db..3edbc4ab98c0 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
@@ -443,7 +443,9 @@ void ASTResultSynthesizer::CommitPersistentDecls() {
 return;
 
   auto *persistent_vars = llvm::cast(state);
-  TypeSystemClang *scratch_ctx = 
ScratchTypeSystemClang::GetForTarget(m_target);
+
+  TypeSystemClang *scratch_ctx = ScratchTypeSystemClang::GetForTarget(
+  m_target, m_ast_context->getLangOpts());
 
   for (clang::NamedDecl *decl : m_decls) {
 StringRef name = decl->getName();

diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
index cf34d9359f11..79ee565a3fdd 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang

[Lldb-commits] [PATCH] D92759: [lldb] Introduce separate scratch ASTs for debug info types and types imported from C++ modules.

2020-12-10 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG47e7ecdd7d36: [lldb] Introduce separate scratch ASTs for 
debug info types and types imported… (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D92759?vs=310535&id=310958#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92759

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  
lldb/test/API/commands/expression/import-std-module/non-module-type-separation/Makefile
  
lldb/test/API/commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py
  
lldb/test/API/commands/expression/import-std-module/non-module-type-separation/main.cpp
  lldb/unittests/Symbol/TestTypeSystemClang.cpp

Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp
===
--- lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -730,3 +730,15 @@
   EXPECT_FALSE(method->isDirectMethod());
   EXPECT_EQ(method->getDeclName().getObjCSelector().getAsString(), "foo");
 }
+
+TEST(TestScratchTypeSystemClang, InferSubASTFromLangOpts) {
+  LangOptions lang_opts;
+  EXPECT_EQ(
+  ScratchTypeSystemClang::DefaultAST,
+  ScratchTypeSystemClang::InferIsolatedASTKindFromLangOpts(lang_opts));
+
+  lang_opts.Modules = true;
+  EXPECT_EQ(
+  ScratchTypeSystemClang::IsolatedASTKind::CppModules,
+  ScratchTypeSystemClang::InferIsolatedASTKindFromLangOpts(lang_opts));
+}
Index: lldb/test/API/commands/expression/import-std-module/non-module-type-separation/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/import-std-module/non-module-type-separation/main.cpp
@@ -0,0 +1,17 @@
+#include 
+
+struct DbgInfoClass {
+  std::vector ints;
+};
+
+int main(int argc, char **argv) {
+  std::vector a = {3, 1, 2};
+
+  // Create a std::vector of a class from debug info with one element.
+  std::vector dbg_info_vec;
+  dbg_info_vec.resize(1);
+  // And that class has a std::vector of integers that comes from the C++
+  // module.
+  dbg_info_vec.back().ints.push_back(1);
+  return 0; // Set break point at this line.
+}
Index: lldb/test/API/commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py
@@ -0,0 +1,88 @@
+"""
+Test that LLDB is separating C++ module types and debug information types
+in the scratch AST.
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@add_test_categories(["libc++"])
+@skipIf(compiler=no_match("clang"))
+def test(self):
+"""
+This test is creating ValueObjects with both C++ module and debug
+info types for std::vector. We can't merge these types into
+the same AST, so for this test to pass LLDB should split the types
+up depending on whether they come from a module or not.
+"""
+self.build()
+
+lldbutil.run_to_source_breakpoint(self,
+  "// Set break point at this line.",
+  lldb.SBFileSpec("main.cpp"))
+
+children = [
+ValueCheck(value="3"),
+ValueCheck(value="1"),
+ValueCheck(value="2"),
+]
+
+# The debug info vector type doesn't know about the default template
+# arguments, so they will be printed.
+debug_info_vector_type = "std::vector >"
+
+# The C++ module vector knows its default template arguments and will
+# suppress them.
+module_vector_type = "std::vector"
+
+# First muddy the scratch AST with non-C++ module types.
+self.expect_expr("a", result_type=debug_info_vector_type,
+ result_children=children)
+self.expect_expr("dbg_info_vec",
+ result_type="std::vector >",
+ result_children=[
+ValueCheck(type="DbgInfoClass", children=[
+ValueCheck(name="ints", type=debug_info_vector_type, children=[
+ValueCheck(value="1")
+  

[Lldb-commits] [lldb] 1eee246 - [lldb] Remove single-case switch statement (NFC)

2020-12-10 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-12-10T10:58:30-08:00
New Revision: 1eee24677bb61a83bcb2f091d7e3e722f782cb25

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

LOG: [lldb] Remove single-case switch statement (NFC)

Use an early continue instead and save a level of indentation. This is a
Maison Riss 2019 vintage, made in France and aged in California.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 2c045d6dc9c0..188a667ca564 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3049,88 +3049,85 @@ DWARFASTParser::ParseChildArrayInfo(const DWARFDIE 
&parent_die,
   for (DWARFDIE die = parent_die.GetFirstChild(); die.IsValid();
die = die.GetSibling()) {
 const dw_tag_t tag = die.Tag();
-switch (tag) {
-case DW_TAG_subrange_type: {
-  DWARFAttributes attributes;
-  const size_t num_child_attributes = die.GetAttributes(attributes);
-  if (num_child_attributes > 0) {
-uint64_t num_elements = 0;
-uint64_t lower_bound = 0;
-uint64_t upper_bound = 0;
-bool upper_bound_valid = false;
-uint32_t i;
-for (i = 0; i < num_child_attributes; ++i) {
-  const dw_attr_t attr = attributes.AttributeAtIndex(i);
-  DWARFFormValue form_value;
-  if (attributes.ExtractFormValueAtIndex(i, form_value)) {
-switch (attr) {
-case DW_AT_name:
-  break;
+if (tag != DW_TAG_subrange_type)
+  continue;
 
-case DW_AT_count:
-  if (DWARFDIE var_die = die.GetReferencedDIE(DW_AT_count)) {
-if (var_die.Tag() == DW_TAG_variable)
-  if (exe_ctx) {
-if (auto frame = exe_ctx->GetFrameSP()) {
-  Status error;
-  lldb::VariableSP var_sp;
-  auto valobj_sp = 
frame->GetValueForVariableExpressionPath(
-  var_die.GetName(), eNoDynamicValues, 0, var_sp,
-  error);
-  if (valobj_sp) {
-num_elements = valobj_sp->GetValueAsUnsigned(0);
-break;
-  }
+DWARFAttributes attributes;
+const size_t num_child_attributes = die.GetAttributes(attributes);
+if (num_child_attributes > 0) {
+  uint64_t num_elements = 0;
+  uint64_t lower_bound = 0;
+  uint64_t upper_bound = 0;
+  bool upper_bound_valid = false;
+  uint32_t i;
+  for (i = 0; i < num_child_attributes; ++i) {
+const dw_attr_t attr = attributes.AttributeAtIndex(i);
+DWARFFormValue form_value;
+if (attributes.ExtractFormValueAtIndex(i, form_value)) {
+  switch (attr) {
+  case DW_AT_name:
+break;
+
+  case DW_AT_count:
+if (DWARFDIE var_die = die.GetReferencedDIE(DW_AT_count)) {
+  if (var_die.Tag() == DW_TAG_variable)
+if (exe_ctx) {
+  if (auto frame = exe_ctx->GetFrameSP()) {
+Status error;
+lldb::VariableSP var_sp;
+auto valobj_sp = frame->GetValueForVariableExpressionPath(
+var_die.GetName(), eNoDynamicValues, 0, var_sp,
+error);
+if (valobj_sp) {
+  num_elements = valobj_sp->GetValueAsUnsigned(0);
+  break;
 }
   }
-  } else
-num_elements = form_value.Unsigned();
-  break;
+}
+} else
+  num_elements = form_value.Unsigned();
+break;
 
-case DW_AT_bit_stride:
-  array_info.bit_stride = form_value.Unsigned();
-  break;
+  case DW_AT_bit_stride:
+array_info.bit_stride = form_value.Unsigned();
+break;
 
-case DW_AT_byte_stride:
-  array_info.byte_stride = form_value.Unsigned();
-  break;
+  case DW_AT_byte_stride:
+array_info.byte_stride = form_value.Unsigned();
+break;
 
-case DW_AT_lower_bound:
-  lower_bound = form_value.Unsigned();
-  break;
+  case DW_AT_lower_bound:
+lower_bound = form_value.Unsigned();
+break;
 
-case DW_AT_upper_bound:
-  upper_bound_valid = true;
-  upper_bound = form_value.Unsigned();
-

[Lldb-commits] [PATCH] D93052: "target create" shouldn't save target if the command failed

2020-12-10 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

It's a little awkward that you have the "do_select" parameter with a default 
argument of "true" but then you explicitly pass "true" every time you use it...

It seems reasonable that the Debugger might want to create targets w/o 
selecting them.  For instance, suppose we had some mechanism to say "attach to 
everything that the currently being debugged process launches", and you were 
stepping over the routine in the main target that creates a new process, we 
will make a new target for the child process, but you really wouldn't want to 
make the new process' target the default target, it hasn't done anything 
interesting yet...  So having the parameter there seems right.  But either use 
the default value or don't have it.  I'm in favor of the latter, it's good for 
people to see that the selected target is going to get switched by this call.

Other than that, this seems fine to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93052

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


[Lldb-commits] [PATCH] D92164: Make CommandInterpreter's execution context the same as debugger's one.

2020-12-10 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

This all looks fine except I'm not sure you can have a single override context. 
 The lldb command line is only a bit recursive, but you can have sequences like:

(lldb) command source file_that_contains_a_step

>>> Step hits a breakpoint which has commands

  One of those commands is a Python command that calls 
SBCommandInterpreter::HandleCommandsFromFile - passing in some other 
SBExecutionContext

IIUC, running the breakpoint command will push one override before running the 
command.  Then HandleCommandsFromFile will push another, and you'll won't get 
back to the first one.

And one of the weaknesses of lldb right now is that commands can't nest as well 
as they should.  For instance, you really should be able to hit a breakpoint, 
and have the breakpoint command able to do a couple of next's of whatever, even 
though the weakness of the lldb command interpreter precludes that at present.

So both for somewhat esoteric reasons like that above, and to future proof this 
change, I think you have to use a stack for the overrides/restore, not a single 
element.


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

https://reviews.llvm.org/D92164

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


[Lldb-commits] [PATCH] D92164: Make CommandInterpreter's execution context the same as debugger's one.

2020-12-10 Thread Tatyana Krasnukha via Phabricator via lldb-commits
tatyana-krasnukha updated this revision to Diff 31.
tatyana-krasnukha added a comment.

Thanks for pointing to the nested command problem! Replaced pointer to the 
execution context with the stack of contexts.


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

https://reviews.llvm.org/D92164

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/source/Breakpoint/BreakpointOptions.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectRegexCommand.cpp
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/python_api/debugger/Makefile
  lldb/test/API/python_api/debugger/TestDebuggerAPI.py
  lldb/test/API/python_api/debugger/main.cpp

Index: lldb/test/API/python_api/debugger/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/debugger/main.cpp
@@ -0,0 +1,9 @@
+// This simple program is to test the lldb Python API SBDebugger.
+
+int func(int val) {
+return val - 1;
+}
+
+int main (int argc, char const *argv[]) {
+return func(argc);
+}
Index: lldb/test/API/python_api/debugger/TestDebuggerAPI.py
===
--- lldb/test/API/python_api/debugger/TestDebuggerAPI.py
+++ lldb/test/API/python_api/debugger/TestDebuggerAPI.py
@@ -43,3 +43,54 @@
 target = lldb.SBTarget()
 self.assertFalse(target.IsValid())
 self.dbg.DeleteTarget(target)
+
+def test_debugger_internal_variable(self):
+"""Ensure that SBDebugger reachs the same instance of properties
+   regardless CommandInterpreter's context initialization"""
+self.build()
+exe = self.getBuildArtifact("a.out")
+
+# Create a target by the debugger.
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+property_name = "target.process.memory-cache-line-size"
+
+def get_cache_line_size():
+value_list = lldb.SBStringList()
+value_list = self.dbg.GetInternalVariableValue(property_name,
+   self.dbg.GetInstanceName())
+
+self.assertEqual(value_list.GetSize(), 1)
+try:
+return int(value_list.GetStringAtIndex(0))
+except ValueError as error:
+self.fail("Value is not a number: " + error)
+
+# Get global property value while there are no processes.
+global_cache_line_size = get_cache_line_size()
+
+# Run a process via SB interface. CommandInterpreter's execution context
+# remains empty.
+error = lldb.SBError()
+launch_info = lldb.SBLaunchInfo(None)
+launch_info.SetLaunchFlags(lldb.eLaunchFlagStopAtEntry)
+process = target.Launch(launch_info, error)
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# This should change the value of a process's local property.
+new_cache_line_size = global_cache_line_size + 512
+error = self.dbg.SetInternalVariable(property_name,
+ str(new_cache_line_size),
+ self.dbg.GetInstanceName())
+self.assertTrue(error.Success(),
+property_name + " value was changed successfully")
+
+# Check that it was set actually.
+self.assertEqual(get_cache_line_size(), new_cache_line_size)
+
+# Run any command to initialize CommandInterpreter's execution context.
+self.runCmd("target list")
+
+# Test the local property again, is it set to new_cache_line_size?
+self.assertEqual(get_cache_line_size(), new_cache_line_size)
Index: lldb/test/API/python_api/debugger/Makefile
===
--- /dev/null
+++ lldb/test/API/python_api/debugger/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3351,7 +3351,7 @@
   // Force Async:
   bool old_async = debugger.GetAsyncExecution();
   debugger.SetAsyncExecution(true);
-  debugger.GetCommandInterpreter().HandleCommands(GetCommands(), &exc_ctx,
+  debugger.GetCommandInterpreter().HandleCommands(GetCommands(), exc_ctx,
   options, result);
   debugger.SetAsyncExecution(old_async);
   lldb::ReturnStatus status = result.GetStatus();
Index: lldb/source/Interpreter/CommandInterpreter.cpp