[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

[Jim Ingham via Phabricator via lldb-commits]

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

Thanks for doing this, it will be useful!

I'll let Jonas comment on whether the Reproducer bindings are right, though it 
looks fine to me.

One of the very common uses of an SBEnvironment is the one in an SBTarget, 
which gets used to create new processes.  With what you have you can get access 
to it, so that's all good.  But you do it by getting the target's platform, and 
then the Environment from the platform.  It might be more discoverable to have 
the SBTarget hand it out directly as well, however.  And there is an 
lldb_private::Target::GetEnvironment, so it's pretty natural.

Also, you need some tests.  You can probably do a lot of it with unit tests.  
Then maybe a small API test that sets environment variables and fetches them 
out this way and checks that they are right.

The added API's are up to you.  The requested changes are for tests...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111



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

[Lldb-commits] [lldb] 57da8f7 - Add support for XFAILing a test based on a setting.

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2020-03-12T19:26:24-07:00
New Revision: 57da8f720ce18100c5c6fb5c2247109e1ef963b5

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

LOG: Add support for XFAILing a test based on a setting.

This is analogous to the skipping mechanism introduced in
https://reviews.llvm.org/D75864

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/test/API/sanity/TestSettingSkipping.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index d2241833f494..cc28ae901634 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -258,7 +258,8 @@ def expectedFailureAll(bugnumber=None,
debug_info=None,
swig_version=None, py_version=None,
macos_version=None,
-   remote=None, dwarf_version=None):
+   remote=None, dwarf_version=None,
+   setting=None):
 return _decorateTest(DecorateMode.Xfail,
  bugnumber=bugnumber,
  oslist=oslist, hostoslist=hostoslist,
@@ -267,7 +268,8 @@ def expectedFailureAll(bugnumber=None,
  debug_info=debug_info,
  swig_version=swig_version, py_version=py_version,
  macos_version=None,
- remote=remote,dwarf_version=dwarf_version)
+ remote=remote,dwarf_version=dwarf_version,
+ setting=setting)
 
 
 # provide a function to skip on defined oslist, compiler version, and archs

diff  --git a/lldb/test/API/sanity/TestSettingSkipping.py 
b/lldb/test/API/sanity/TestSettingSkipping.py
index 10e7144a0068..206c7b4ecd73 100644
--- a/lldb/test/API/sanity/TestSettingSkipping.py
+++ b/lldb/test/API/sanity/TestSettingSkipping.py
@@ -27,3 +27,11 @@ def testNoMatch(self):
   def testNotExisting(self):
 self.assertTrue(True, "This test should run!")
 
+  @expectedFailureAll(setting=('target.prefer-dynamic-value', 
'no-dynamic-values'))
+  def testXFAIL(self):
+self.assertTrue(False, "This test should run and fail!")
+
+  @expectedFailureAll(setting=('target.prefer-dynamic-value', 'run-target'))
+  def testNotXFAIL(self):
+self.assertTrue(True, "This test should run and succeed!")
+



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

[Lldb-commits] [PATCH] D76080: Adjust error_msg handling for expect_expr in lldbtest.py

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik abandoned this revision.
shafik added a comment.

After discussing this @teemperor in some detail it looks like getting the 
`error_msg` case to work well is not a trivial task. So for these cases we 
should revert to just using `expect`. I think the plan is that he will remove 
the feature for now.


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

https://reviews.llvm.org/D76080



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

[Lldb-commits] [lldb] a9682cc - Convert settings list into a tuple so it can be matched by the decorator.

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2020-03-12T17:51:15-07:00
New Revision: a9682ccb7e70a036bd3acbbe97ed8ab74a7732d3

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

LOG: Convert settings list into a tuple so it can be matched by the decorator.

Added: 


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

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 991e29d7e6b0..4e86d1a59322 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -349,7 +349,8 @@ def parseOptionsAndInitTestdirs():
 logging.error('"%s" is not a setting in the form "key=value"',
   setting[0])
 sys.exit(-1)
-configuration.settings.append(setting[0].split('=', 1))
+setting_list = setting[0].split('=', 1)
+configuration.settings.append((setting_list[0], setting_list[1]))
 
 if args.d:
 sys.stdout.write(



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

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 250108.
wallace added a comment.

format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBEnvironment.i
  lldb/bindings/interface/SBPlatform.i
  lldb/bindings/interfaces.swig
  lldb/include/lldb/API/LLDB.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBEnvironment.h
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBEnvironment.cpp
  lldb/source/API/SBPlatform.cpp

Index: lldb/source/API/SBPlatform.cpp
===
--- lldb/source/API/SBPlatform.cpp
+++ lldb/source/API/SBPlatform.cpp
@@ -8,9 +8,11 @@
 
 #include "lldb/API/SBPlatform.h"
 #include "SBReproducerPrivate.h"
+#include "lldb/API/SBEnvironment.h"
 #include "lldb/API/SBError.h"
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBLaunchInfo.h"
+#include "lldb/API/SBPlatform.h"
 #include "lldb/API/SBUnixSignals.h"
 #include "lldb/Host/File.h"
 #include "lldb/Target/Platform.h"
@@ -649,6 +651,17 @@
   return LLDB_RECORD_RESULT(SBUnixSignals());
 }
 
+SBEnvironment SBPlatform::GetEnvironment() {
+  LLDB_RECORD_METHOD_NO_ARGS(lldb::SBEnvironment, SBPlatform, GetEnvironment);
+  PlatformSP platform_sp(GetSP());
+
+  if (platform_sp) {
+return LLDB_RECORD_RESULT(SBEnvironment(platform_sp->GetEnvironment()));
+  }
+
+  return LLDB_RECORD_RESULT(SBEnvironment());
+}
+
 namespace lldb_private {
 namespace repro {
 
@@ -740,6 +753,7 @@
(const char *));
   LLDB_REGISTER_METHOD(lldb::SBError, SBPlatform, SetFilePermissions,
(const char *, uint32_t));
+  LLDB_REGISTER_METHOD(lldb::SBEnvironment, SBPlatform, GetEnvironment, ());
   LLDB_REGISTER_METHOD_CONST(lldb::SBUnixSignals, SBPlatform, GetUnixSignals,
  ());
 }
Index: lldb/source/API/SBEnvironment.cpp
===
--- /dev/null
+++ lldb/source/API/SBEnvironment.cpp
@@ -0,0 +1,69 @@
+//===-- SBEnvironment.cpp
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/API/SBEnvironment.h"
+#include "SBReproducerPrivate.h"
+#include "Utils.h"
+#include "lldb/Utility/Environment.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+SBEnvironment::SBEnvironment() : m_opaque_up(new Environment()) {
+  LLDB_RECORD_CONSTRUCTOR_NO_ARGS(SBEnvironment);
+}
+
+SBEnvironment::SBEnvironment(const SBEnvironment )
+: m_opaque_up(new Environment()) {
+  LLDB_RECORD_CONSTRUCTOR(SBEnvironment, (const lldb::SBEnvironment &), rhs);
+
+  m_opaque_up = clone(rhs.m_opaque_up);
+}
+
+SBEnvironment::SBEnvironment(const Environment )
+: m_opaque_up(new Environment()) {
+  *m_opaque_up = rhs;
+}
+
+SBEnvironment::~SBEnvironment() = default;
+
+const SBEnvironment ::operator=(const SBEnvironment ) {
+  LLDB_RECORD_METHOD(const lldb::SBEnvironment &,
+ SBEnvironment, operator=,(const lldb::SBEnvironment &),
+ rhs);
+
+  if (this != )
+m_opaque_up = clone(rhs.m_opaque_up);
+  return LLDB_RECORD_RESULT(*this);
+}
+
+int SBEnvironment::Size() {
+  LLDB_RECORD_METHOD_NO_ARGS(int, SBEnvironment, Size);
+  return LLDB_RECORD_RESULT(m_opaque_up->size());
+}
+
+const char *SBEnvironment::GetEntryAtIndex(int idx) {
+  LLDB_RECORD_METHOD(const char *, SBEnvironment, GetEntryAtIndex, (int), idx);
+  return LLDB_RECORD_RESULT(m_opaque_up->getEnvp().get()[idx]);
+}
+
+namespace lldb_private {
+namespace repro {
+
+template <> void RegisterMethods(Registry ) {
+  LLDB_REGISTER_CONSTRUCTOR(SBEnvironment, ());
+  LLDB_REGISTER_CONSTRUCTOR(SBEnvironment, (const lldb::SBEnvironment &));
+  LLDB_REGISTER_METHOD(const lldb::SBEnvironment &,
+   SBEnvironment, operator=,(const lldb::SBEnvironment &));
+  LLDB_REGISTER_METHOD(int, SBEnvironment, Size, ());
+  LLDB_REGISTER_METHOD(const char *, SBEnvironment, GetEntryAtIndex, (int));
+}
+
+} // namespace repro
+} // namespace lldb_private
Index: lldb/source/API/CMakeLists.txt
===
--- lldb/source/API/CMakeLists.txt
+++ lldb/source/API/CMakeLists.txt
@@ -35,6 +35,7 @@
   SBData.cpp
   SBDebugger.cpp
   SBDeclaration.cpp
+  SBEnvironment.cpp
   SBError.cpp
   SBEvent.cpp
   SBExecutionContext.cpp
Index: lldb/include/lldb/lldb-forward.h
===
--- lldb/include/lldb/lldb-forward.h
+++ lldb/include/lldb/lldb-forward.h
@@ -76,6 +76,7 @@
 

[Lldb-commits] [PATCH] D76105: [lldb/settings] Reset the inferior environment when target.inherit-env is toggled

[Frederic Riss via Phabricator via lldb-commits]

friss added a comment.

In D76105#1920633 , @jingham wrote:

> If I'm following the logic correctly, if you run a target with inherit-env 
> off, and then do:
>
>   env VAR_IN_ENVIRONMENT=NOT_THE_ENVIRONMENTS_VALUE
>  
>
>
> then turn inherit-env on, we will preserve the value you set it to, not the 
> environment value, because you pass in false for can_replace.  That seems 
> right to me.  It would be good to test that case explicitly, however.
>
> But if you have inherit-env on and then run the command above, and then turn 
> inherit-env off, your changed value will just get deleted.  That seems 
> unexpected.  I think you have to compare the values in the else branch and 
> only delete the key if the value is the same as the environment value.  Does 
> that sound right?


Yeah, this shortcoming was pretty obvious while writing the patch. I don't like 
it very much, it seems like the inherit behavior should be handled closer to 
the point we launch, Or at least the inherited environment should be stored 
separately from the one you set manually. Comparing values would solve one 
class of issues, but what about explicitly setting one variable to the same 
value it has in your environment. Then you would delete it when changing the 
inherit-env property. Also not very intuitive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76105



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

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: labath, clayborg.
Herald added subscribers: lldb-commits, mgorny.
Herald added a project: LLDB.

Inspired by https://reviews.llvm.org/D74636, I'm introducing a basic version of 
Environment in the API. More functionalities can be added as needed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76111

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBEnvironment.i
  lldb/bindings/interface/SBPlatform.i
  lldb/bindings/interfaces.swig
  lldb/include/lldb/API/LLDB.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBEnvironment.h
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBEnvironment.cpp
  lldb/source/API/SBPlatform.cpp

Index: lldb/source/API/SBPlatform.cpp
===
--- lldb/source/API/SBPlatform.cpp
+++ lldb/source/API/SBPlatform.cpp
@@ -8,9 +8,11 @@
 
 #include "lldb/API/SBPlatform.h"
 #include "SBReproducerPrivate.h"
+#include "lldb/API/SBEnvironment.h"
 #include "lldb/API/SBError.h"
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBLaunchInfo.h"
+#include "lldb/API/SBPlatform.h"
 #include "lldb/API/SBUnixSignals.h"
 #include "lldb/Host/File.h"
 #include "lldb/Target/Platform.h"
@@ -649,6 +651,17 @@
   return LLDB_RECORD_RESULT(SBUnixSignals());
 }
 
+SBEnvironment SBPlatform::GetEnvironment() {
+  LLDB_RECORD_METHOD_NO_ARGS(lldb::SBEnvironment, SBPlatform, GetEnvironment);
+  PlatformSP platform_sp(GetSP());
+
+  if (platform_sp) {
+return LLDB_RECORD_RESULT(SBEnvironment(platform_sp->GetEnvironment()));
+  }
+
+  return LLDB_RECORD_RESULT(SBEnvironment());
+}
+
 namespace lldb_private {
 namespace repro {
 
@@ -740,6 +753,7 @@
(const char *));
   LLDB_REGISTER_METHOD(lldb::SBError, SBPlatform, SetFilePermissions,
(const char *, uint32_t));
+  LLDB_REGISTER_METHOD(lldb::SBEnvironment, SBPlatform, GetEnvironment, ()); 
   LLDB_REGISTER_METHOD_CONST(lldb::SBUnixSignals, SBPlatform, GetUnixSignals,
  ());
 }
Index: lldb/source/API/SBEnvironment.cpp
===
--- /dev/null
+++ lldb/source/API/SBEnvironment.cpp
@@ -0,0 +1,66 @@
+//===-- SBEnvironment.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/API/SBEnvironment.h"
+#include "SBReproducerPrivate.h"
+#include "Utils.h"
+#include "lldb/Utility/Environment.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+SBEnvironment::SBEnvironment() : m_opaque_up(new Environment()) {
+  LLDB_RECORD_CONSTRUCTOR_NO_ARGS(SBEnvironment);
+}
+
+SBEnvironment::SBEnvironment(const SBEnvironment ) : m_opaque_up(new Environment()) {
+  LLDB_RECORD_CONSTRUCTOR(SBEnvironment, (const lldb::SBEnvironment &), rhs);
+
+  m_opaque_up = clone(rhs.m_opaque_up);
+}
+
+SBEnvironment::SBEnvironment(const Environment ) : m_opaque_up(new Environment()) {
+  *m_opaque_up = rhs; 
+}
+
+SBEnvironment::~SBEnvironment() = default;
+
+const SBEnvironment ::operator=(const SBEnvironment ) {
+  LLDB_RECORD_METHOD(const lldb::SBEnvironment &,
+ SBEnvironment, operator=,(const lldb::SBEnvironment &), rhs);
+
+  if (this != )
+m_opaque_up = clone(rhs.m_opaque_up);
+  return LLDB_RECORD_RESULT(*this);
+}
+
+int SBEnvironment::Size() {
+  LLDB_RECORD_METHOD_NO_ARGS(int, SBEnvironment, Size);
+  return LLDB_RECORD_RESULT(m_opaque_up->size());
+}
+
+const char *SBEnvironment::GetEntryAtIndex(int idx) {
+  LLDB_RECORD_METHOD(const char *, SBEnvironment, GetEntryAtIndex, (int), idx);
+  return LLDB_RECORD_RESULT(m_opaque_up->getEnvp().get()[idx]);
+}
+
+namespace lldb_private {
+namespace repro {
+
+template <>
+void RegisterMethods(Registry ) {
+  LLDB_REGISTER_CONSTRUCTOR(SBEnvironment, ());
+  LLDB_REGISTER_CONSTRUCTOR(SBEnvironment, (const lldb::SBEnvironment &));
+  LLDB_REGISTER_METHOD(const lldb::SBEnvironment &,
+   SBEnvironment, operator=,(const lldb::SBEnvironment &));
+  LLDB_REGISTER_METHOD(int, SBEnvironment, Size, ()); 
+  LLDB_REGISTER_METHOD(const char *, SBEnvironment, GetEntryAtIndex, (int));
+}
+
+}
+}
Index: lldb/source/API/CMakeLists.txt
===
--- lldb/source/API/CMakeLists.txt
+++ lldb/source/API/CMakeLists.txt
@@ -35,6 +35,7 @@
   SBData.cpp
   SBDebugger.cpp
   SBDeclaration.cpp
+  SBEnvironment.cpp
   SBError.cpp
   SBEvent.cpp
   SBExecutionContext.cpp
Index: lldb/include/lldb/lldb-forward.h

[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D75711#1920642 , @clayborg wrote:

> Everything looks good, just a question in inlined comment about having a 
> thread plan hold onto a pointer to a thread. Seems dangerous


The way the ThreadPlanStacks will get used, every time a process stops, lldb 
will build up a new thread list like it does already, then we update the 
collection of thread plan stacks, passing in the new thread list.  That will 
grab the TID for each new thread and look it up in the map of stacks (which is 
indexed by TID).  Each time we find a TID from the new thread list that's 
already in the map, we set the Thread * in the stack map to the one we got the 
TID from.  Then for all the TID's that aren't in the new thread list we either 
discard it (if we're not holding onto plans for vanished threads) or we set its 
Thread * to nullptr.  So the Thread * is just acting as a cache so you don't 
need to keep looking up the TID every time you need to get the Thread * for a 
ThreadPlan between one stop and another.

Unless there's some way that a thread that we saw when we stopped can go away 
before the next resume without the ThreadList being notified so it can re-sync, 
this mechanism is safe, and I can't think of any mechanism of that sort in lldb.

If you're still worried about this, I think the safe solution is to remove the 
Thread * and look up the TID in the Process every time you need the Thread *.  
Using a ThreadWP doesn't really express what you want here, because the Thread 
may have been discarded but it's SP is still alive because it's in some list we 
haven't gotten rid of or something like that.  So WP.lock() working doesn't 
really mean the thread is still alive in the process, but TID in ThreadList 
does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75711



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

[Lldb-commits] [lldb] af7fc8c - [lldb] Remove unused and too strict error_msg parameter from expect_expr

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-03-13T00:45:43+01:00
New Revision: af7fc8c1bbcb380610451be59c022595bd7ba4bd

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

LOG: [lldb] Remove unused and too strict error_msg parameter from expect_expr

Directly matching the error message is nearly never useful. We can re-add
error-checking once we have a plan to properly implement this.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index cd48747c3557..f8f916036f9a 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2396,7 +2396,6 @@ def expect_expr(
 result_summary=None,
 result_value=None,
 result_type=None,
-error_msg=None,
 ):
 """
 Evaluates the given expression and verifies the result.
@@ -2404,7 +2403,6 @@ def expect_expr(
 :param result_summary: The summary that the expression should have. 
None if the summary should not be checked.
 :param result_value: The value that the expression should have. None 
if the value should not be checked.
 :param result_type: The type that the expression result should have. 
None if the type should not be checked.
-:param error_msg: The error message the expression should return. None 
if the error output should not be checked.
 """
 self.assertTrue(expr.strip() == expr, "Expression contains 
trailing/leading whitespace: '" + expr + "'")
 
@@ -2420,11 +2418,6 @@ def expect_expr(
 
 eval_result = frame.EvaluateExpression(expr, options)
 
-if error_msg:
-self.assertFalse(eval_result.IsValid(), "Unexpected success with 
result: '" + str(eval_result) + "'")
-self.assertEqual(error_msg, eval_result.GetError().GetCString())
-return
-
 if not eval_result.GetError().Success():
 self.assertTrue(eval_result.GetError().Success(),
 "Unexpected failure with msg: " + 
eval_result.GetError().GetCString())



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

[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Everything looks good, just a question in inlined comment about having a thread 
plan hold onto a pointer to a thread. Seems dangerous




Comment at: lldb/include/lldb/Target/ThreadPlan.h:601
 
+  Thread *m_thread;
   ThreadPlanKind m_kind;

This seems dangerous to hold onto? It can go stale if the thread goes away 
right? If we have the m_tid, why do we need this? If we want a thread pointer 
in this class, should it be a std::weak_ptr? It also seems that if we make a 
ThreadRef class that is something like:

```
struct ThreadRef {
  std::weak_ptr m_thread_wp;
  lldb::tid_t m_tid;
};
```

We could re-use this in this class and ExecutionContextRef.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75711



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

[Lldb-commits] [PATCH] D75761: [lldb] Fix to get the AST we generate for function templates to be closer to what clang generates and expects

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

FWIW, I don't think the error cases should use expect_expr as it currently 
doesn't have a reasonable way to handle errors (the error_msg thing is really 
too strict and didn't land on purpose. That was more of a side effect of all 
the refactoring that went into the expect_expr patch).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D75761



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

[Lldb-commits] [PATCH] D76105: [lldb/settings] Reset the inferior environment when target.inherit-env is toggled

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

If I'm following the logic correctly, if you run a target with inherit-env off, 
and then do:

  env VAR_IN_ENVIRONMENT=NOT_THE_ENVIRONMENTS_VALUE

then turn inherit-env on, we will preserve the value you set it to, not the 
environment value, because you pass in false for can_replace.  That seems right 
to me.  It would be good to test that case explicitly, however.

But if you have inherit-env on and then run the command above, and then turn 
inherit-env off, your changed value will just get deleted.  That seems 
unexpected.  I think you have to compare the values in the else branch and only 
delete the key if the value is the same as the environment value.  Does that 
sound right?

If so, then it would be good to test that as well...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76105



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

[Lldb-commits] [PATCH] D76080: Adjust error_msg handling for expect_expr in lldbtest.py

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

In D76080#1920483 , @teemperor wrote:

> - All of the asserts should print a useful error when failing (i.e., one that 
> allows us to directly write a fix). You could do assertIn which is clearer 
> than `find(...)` and automatically gives an error message that is useful. For 
> the assertTrue just print the unexpected result in the assert message as the 
> old code did.


Using `assertIn` makes sense.

We are using `assertIn` in a few places and what we are looking for in those is 
not very helpful for guidance.


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

https://reviews.llvm.org/D76080



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

[Lldb-commits] [PATCH] D76045: [lldb/API] Make Launch(Simple) use args and env from target properties

[Frederic Riss via Phabricator via lldb-commits]

friss added a comment.

This is somewhat useless without D76009  
because the default LaunchInfo doesn't get populated with the environment 
without it. I'll wait for a resolution there before landing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76045



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

[Lldb-commits] [PATCH] D76080: Adjust error_msg handling for expect_expr in lldbtest.py

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 250083.
shafik added a comment.

Moving to using `assertIn` as suggest in comment.


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

https://reviews.llvm.org/D76080

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


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2421,8 +2421,9 @@
 eval_result = frame.EvaluateExpression(expr, options)
 
 if error_msg:
-self.assertFalse(eval_result.IsValid(), "Unexpected success with 
result: '" + str(eval_result) + "'")
-self.assertEqual(error_msg, eval_result.GetError().GetCString())
+self.assertTrue(eval_result.IsValid(), "Result is not valid")
+self.assertTrue(eval_result.GetError().Fail(), "Unexpected 
success...")
+self.assertIn(error_msg,eval_result.GetError().GetCString())
 return
 
 if not eval_result.GetError().Success():


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2421,8 +2421,9 @@
 eval_result = frame.EvaluateExpression(expr, options)
 
 if error_msg:
-self.assertFalse(eval_result.IsValid(), "Unexpected success with result: '" + str(eval_result) + "'")
-self.assertEqual(error_msg, eval_result.GetError().GetCString())
+self.assertTrue(eval_result.IsValid(), "Result is not valid")
+self.assertTrue(eval_result.GetError().Fail(), "Unexpected success...")
+self.assertIn(error_msg,eval_result.GetError().GetCString())
 return
 
 if not eval_result.GetError().Success():
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76105: [lldb/settings] Reset the inferior environment when target.inherit-env is toggled

[Frederic Riss via Phabricator via lldb-commits]

friss created this revision.
friss added reviewers: labath, jingham.
Herald added a project: LLDB.

Allow the target.env-vars property to be recomputed when the
target.inherit-env property changes. This allows to change
the value of the property between runs and get a meaningful
behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76105

Files:
  lldb/source/Target/Target.cpp
  lldb/test/API/commands/settings/TestSettings.py


Index: lldb/test/API/commands/settings/TestSettings.py
===
--- lldb/test/API/commands/settings/TestSettings.py
+++ lldb/test/API/commands/settings/TestSettings.py
@@ -293,6 +293,23 @@
 "The host environment variable 'MY_HOST_ENV_VAR1' successfully 
passed.",
 "The host environment variable 'MY_HOST_ENV_VAR2' successfully 
passed."])
 
+self.runCmd('settings set target.inherit-env false')
+
+self.addTearDownHook(unset_env_variables)
+self.runCmd("process launch --working-dir 
'{0}'".format(self.get_process_working_directory()),
+RUN_SUCCEEDED)
+
+# Read the output file produced by running the program.
+output = lldbutil.read_file_from_process_wd(self, "output1.txt")
+
+self.expect(
+output,
+matching=False,
+exe=False,
+substrs=[
+"The host environment variable 'MY_HOST_ENV_VAR1' successfully 
passed.",
+"The host environment variable 'MY_HOST_ENV_VAR2' successfully 
passed."])
+
 @skipIfDarwinEmbedded   #  debugserver on ios etc 
can't write files
 def test_set_error_output_path(self):
 """Test that setting target.error/output-path for the launched process 
works."""
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3394,36 +3394,43 @@
 
 protected:
   void GetHostEnvironmentIfNeeded() const {
-if (!m_got_host_env) {
-  if (m_target) {
-m_got_host_env = true;
-const uint32_t idx = ePropertyInheritEnv;
-if (GetPropertyAtIndexAsBoolean(
-nullptr, idx, g_target_properties[idx].default_uint_value != 
0)) {
-  PlatformSP platform_sp(m_target->GetPlatform());
-  if (platform_sp) {
-Environment env = platform_sp->GetEnvironment();
-OptionValueDictionary *env_dict =
-GetPropertyAtIndexAsOptionValueDictionary(nullptr,
-  ePropertyEnvVars);
-if (env_dict) {
-  const bool can_replace = false;
-  for (const auto  : env) {
-// Don't allow existing keys to be replaced with ones we get
-// from the platform environment
-env_dict->SetValueForKey(
-ConstString(KV.first()),
-OptionValueSP(new OptionValueString(KV.second.c_str())),
-can_replace);
-  }
-}
-  }
+if (!m_target)
+  return;
+
+const uint32_t idx = ePropertyInheritEnv;
+bool should_inherit = GetPropertyAtIndexAsBoolean(
+nullptr, idx, g_target_properties[idx].default_uint_value != 0);
+
+if (!m_got_host_env || should_inherit != m_host_env_inherited) {
+  m_got_host_env = true;
+  PlatformSP platform_sp(m_target->GetPlatform());
+  if (platform_sp) {
+m_host_env_inherited = should_inherit;
+
+Environment env = platform_sp->GetEnvironment();
+OptionValueDictionary *env_dict =
+GetPropertyAtIndexAsOptionValueDictionary(nullptr,
+  ePropertyEnvVars);
+assert(env_dict && "The target.env-vars property doesn't exist.");
+if (should_inherit) {
+  // Don't allow existing keys to be replaced with ones we get
+  // from the platform environment
+  const bool can_replace = false;
+  for (const auto  : env)
+env_dict->SetValueForKey(
+ConstString(KV.first()),
+OptionValueSP(new OptionValueString(KV.second.c_str())),
+can_replace);
+} else {
+  for (const auto  : env)
+env_dict->DeleteValueForKey(ConstString(KV.first()));
 }
   }
 }
   }
   Target *m_target;
   mutable bool m_got_host_env;
+  mutable bool m_host_env_inherited;
 };
 
 // TargetProperties


Index: lldb/test/API/commands/settings/TestSettings.py
===
--- lldb/test/API/commands/settings/TestSettings.py
+++ lldb/test/API/commands/settings/TestSettings.py
@@ -293,6 +293,23 @@
 "The host environment variable 'MY_HOST_ENV_VAR1' successfully passed.",
 "The host environment variable 

[Lldb-commits] [PATCH] D76009: [lldb/Target] Initialize new targets environment variables from target.env-vars

[Frederic Riss via Phabricator via lldb-commits]

friss added a comment.

I put up a "fix" for the inherit-env issue mentioned here: 
https://reviews.llvm.org/D76105
It is mostly orthogonal to this patch as Jim showed the behavior today is 
already broken.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76009



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

[Lldb-commits] [PATCH] D76080: Adjust error_msg handling for expect_expr in lldbtest.py

[Raphael Isemann via Phabricator via lldb-commits]

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

- All of the asserts should print a useful error when failing (i.e., one that 
allows us to directly write a fix). You could do assertIn which is clearer than 
`find(...)` and automatically gives an error message that is useful. For the 
assertTrue just print the unexpected result in the assert message as the old 
code did.
- Just doing a plain `find` on a single string is a step backwards from the 
substr list in plain `expect`. The whole idea of the expect_expr is to have an 
API that is less prone to unintentionally passing tests and encouraging tests 
that are as strict as possible.

IMHO something similar to Clang's diagnostic verifier 
 
could be nice. So passing a list of expected errors and warnings that we expect 
and unexpected errors/warnings cause a test failure. The warnings themselves 
could be an ordered list of substrs (that's what Clang is doing IIRC). We don't 
have proper "error:" or "warning:" labels in all errors yet IIRC, but otherwise 
I guess that could work?

Maybe it makes sense to check the current error cases we have and see what we 
are usually testing for (I assume it's mostly Clang diagnostics).


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

https://reviews.llvm.org/D76080



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

[Lldb-commits] [lldb] 2411f56 - [lldb/Host] Fix the Windows build

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-03-12T15:31:09-07:00
New Revision: 2411f56bfd1c36f30239c832b75e094d927ee219

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

LOG: [lldb/Host] Fix the Windows build

Update use of ProcessInstanceInfoList which is now a std::vector.

Added: 


Modified: 
lldb/source/Host/windows/Host.cpp

Removed: 




diff  --git a/lldb/source/Host/windows/Host.cpp 
b/lldb/source/Host/windows/Host.cpp
index 28e78885519a..29c96560af25 100644
--- a/lldb/source/Host/windows/Host.cpp
+++ b/lldb/source/Host/windows/Host.cpp
@@ -133,7 +133,7 @@ FileSpec Host::GetModuleFileSpecForHostAddress(const void 
*host_addr) {
 
 uint32_t Host::FindProcesses(const ProcessInstanceInfoMatch _info,
  ProcessInstanceInfoList _infos) {
-  process_infos.Clear();
+  process_infos.clear();
 
   AutoHandle snapshot(CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0));
   if (!snapshot.IsValid())



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

[Lldb-commits] [PATCH] D76004: [lldb] Add YAML traits for ArchSpec and ProcessInstanceInfo

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0ce3b710b49c: [lldb] Add YAML traits for ArchSpec and 
ProcessInstanceInfo (authored by JDevlieghere).

Changed prior to commit:
  https://reviews.llvm.org/D76004?vs=249657=250069#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76004

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/include/lldb/Utility/ProcessInfo.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/source/Utility/ProcessInfo.cpp
  lldb/unittests/Utility/ArchSpecTest.cpp
  lldb/unittests/Utility/ProcessInstanceInfoTest.cpp

Index: lldb/unittests/Utility/ProcessInstanceInfoTest.cpp
===
--- lldb/unittests/Utility/ProcessInstanceInfoTest.cpp
+++ lldb/unittests/Utility/ProcessInstanceInfoTest.cpp
@@ -108,3 +108,60 @@
   EXPECT_TRUE(match.Matches(info_bar));
   EXPECT_TRUE(match.Matches(info_empty));
 }
+
+TEST(ProcessInstanceInfo, Yaml) {
+  std::string buffer;
+  llvm::raw_string_ostream os(buffer);
+
+  // Serialize.
+  ProcessInstanceInfo info("a.out", ArchSpec("x86_64-pc-linux"), 47);
+  info.SetUserID(1);
+  info.SetEffectiveUserID(2);
+  info.SetGroupID(3);
+  info.SetEffectiveGroupID(4);
+  llvm::yaml::Output yout(os);
+  yout << info;
+  os.flush();
+
+  // Deserialize.
+  ProcessInstanceInfo deserialized;
+  llvm::yaml::Input yin(buffer);
+  yin >> deserialized;
+
+  EXPECT_EQ(deserialized.GetNameAsStringRef(), info.GetNameAsStringRef());
+  EXPECT_EQ(deserialized.GetArchitecture(), info.GetArchitecture());
+  EXPECT_EQ(deserialized.GetUserID(), info.GetUserID());
+  EXPECT_EQ(deserialized.GetGroupID(), info.GetGroupID());
+  EXPECT_EQ(deserialized.GetEffectiveUserID(), info.GetEffectiveUserID());
+  EXPECT_EQ(deserialized.GetEffectiveGroupID(), info.GetEffectiveGroupID());
+}
+
+TEST(ProcessInstanceInfoList, Yaml) {
+  std::string buffer;
+  llvm::raw_string_ostream os(buffer);
+
+  // Serialize.
+  ProcessInstanceInfo info("a.out", ArchSpec("x86_64-pc-linux"), 47);
+  info.SetUserID(1);
+  info.SetEffectiveUserID(2);
+  info.SetGroupID(3);
+  info.SetEffectiveGroupID(4);
+  ProcessInstanceInfoList list;
+  list.push_back(info);
+  llvm::yaml::Output yout(os);
+  yout << list;
+  os.flush();
+
+  // Deserialize.
+  ProcessInstanceInfoList deserialized;
+  llvm::yaml::Input yin(buffer);
+  yin >> deserialized;
+
+  ASSERT_EQ(deserialized.size(), static_cast(1));
+  EXPECT_EQ(deserialized[0].GetNameAsStringRef(), info.GetNameAsStringRef());
+  EXPECT_EQ(deserialized[0].GetArchitecture(), info.GetArchitecture());
+  EXPECT_EQ(deserialized[0].GetUserID(), info.GetUserID());
+  EXPECT_EQ(deserialized[0].GetGroupID(), info.GetGroupID());
+  EXPECT_EQ(deserialized[0].GetEffectiveUserID(), info.GetEffectiveUserID());
+  EXPECT_EQ(deserialized[0].GetEffectiveGroupID(), info.GetEffectiveGroupID());
+}
Index: lldb/unittests/Utility/ArchSpecTest.cpp
===
--- lldb/unittests/Utility/ArchSpecTest.cpp
+++ lldb/unittests/Utility/ArchSpecTest.cpp
@@ -9,8 +9,9 @@
 #include "gtest/gtest.h"
 
 #include "lldb/Utility/ArchSpec.h"
-#include "llvm/BinaryFormat/MachO.h"
 #include "llvm/BinaryFormat/ELF.h"
+#include "llvm/BinaryFormat/MachO.h"
+#include "llvm/Support/YAMLParser.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -200,14 +201,14 @@
 
 EXPECT_TRUE(A.IsValid());
 EXPECT_TRUE(B.IsValid());
-
+
 EXPECT_EQ(llvm::Triple::ArchType::arm, B.GetTriple().getArch());
 EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
   B.GetTriple().getVendor());
 EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
 EXPECT_EQ(llvm::Triple::EnvironmentType::UnknownEnvironment,
   B.GetTriple().getEnvironment());
-
+
 A.MergeFrom(B);
 EXPECT_EQ(llvm::Triple::ArchType::arm, A.GetTriple().getArch());
 EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
@@ -406,3 +407,23 @@
 ASSERT_TRUE(D.TripleEnvironmentWasSpecified());
   }
 }
+
+TEST(ArchSpecTest, YAML) {
+  std::string buffer;
+  llvm::raw_string_ostream os(buffer);
+
+  // Serialize.
+  llvm::yaml::Output yout(os);
+  std::vector archs = {ArchSpec("x86_64-pc-linux"),
+ ArchSpec("x86_64-apple-macosx10.12"),
+ ArchSpec("i686-pc-windows")};
+  yout << archs;
+  os.flush();
+
+  // Deserialize.
+  std::vector deserialized;
+  llvm::yaml::Input yin(buffer);
+  yin >> deserialized;
+
+  EXPECT_EQ(archs, deserialized);
+}
Index: lldb/source/Utility/ProcessInfo.cpp
===
--- lldb/source/Utility/ProcessInfo.cpp
+++ lldb/source/Utility/ProcessInfo.cpp
@@ -331,3 +331,16 @@
   m_name_match_type = NameMatch::Ignore;
   m_match_all_users = false;
 }
+
+void llvm::yaml::MappingTraits::mapping(
+IO , 

[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Is this one okay as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75711



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

[Lldb-commits] [lldb] 0ce3b71 - [lldb] Add YAML traits for ArchSpec and ProcessInstanceInfo

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-03-12T14:38:37-07:00
New Revision: 0ce3b710b49c7b9ab837d220547aec92564dd78d

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

LOG: [lldb] Add YAML traits for ArchSpec and ProcessInstanceInfo

Add YAML traits for ArchSpec and ProcessInstanceInfo so they can be
serialized for the reproducers.

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

Added: 


Modified: 
lldb/include/lldb/Utility/ArchSpec.h
lldb/include/lldb/Utility/ProcessInfo.h
lldb/source/Utility/ArchSpec.cpp
lldb/source/Utility/ProcessInfo.cpp
lldb/unittests/Utility/ArchSpecTest.cpp
lldb/unittests/Utility/ProcessInstanceInfoTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/ArchSpec.h 
b/lldb/include/lldb/Utility/ArchSpec.h
index ca6d7e0ca0e4..5466e573c1a5 100644
--- a/lldb/include/lldb/Utility/ArchSpec.h
+++ b/lldb/include/lldb/Utility/ArchSpec.h
@@ -16,6 +16,7 @@
 #include "lldb/lldb-private-enumerations.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Support/YAMLTraits.h"
 #include 
 #include 
 #include 
@@ -541,4 +542,16 @@ bool ParseMachCPUDashSubtypeTriple(llvm::StringRef 
triple_str, ArchSpec );
 
 } // namespace lldb_private
 
+namespace llvm {
+namespace yaml {
+template <> struct ScalarTraits {
+  static void output(const lldb_private::ArchSpec &, void *, raw_ostream &);
+  static StringRef input(StringRef, void *, lldb_private::ArchSpec &);
+  static QuotingType mustQuote(StringRef S) { return QuotingType::Double; }
+};
+} // namespace yaml
+} // namespace llvm
+
+LLVM_YAML_IS_SEQUENCE_VECTOR(lldb_private::ArchSpec)
+
 #endif // LLDB_UTILITY_ARCHSPEC_H

diff  --git a/lldb/include/lldb/Utility/ProcessInfo.h 
b/lldb/include/lldb/Utility/ProcessInfo.h
index d2ad40487224..0d631d06d2df 100644
--- a/lldb/include/lldb/Utility/ProcessInfo.h
+++ b/lldb/include/lldb/Utility/ProcessInfo.h
@@ -9,13 +9,12 @@
 #ifndef LLDB_UTILITY_PROCESSINFO_H
 #define LLDB_UTILITY_PROCESSINFO_H
 
-// LLDB headers
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/Environment.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/NameMatches.h"
-
+#include "llvm/Support/YAMLTraits.h"
 #include 
 
 namespace lldb_private {
@@ -89,6 +88,7 @@ class ProcessInfo {
   const Environment () const { return m_environment; }
 
 protected:
+  template  friend struct llvm::yaml::MappingTraits;
   FileSpec m_executable;
   std::string m_arg0; // argv[0] if supported. If empty, then use m_executable.
   // Not all process plug-ins support specifying an argv[0] that 
diff ers from
@@ -150,6 +150,7 @@ class ProcessInstanceInfo : public ProcessInfo {
   bool verbose) const;
 
 protected:
+  friend struct llvm::yaml::MappingTraits;
   uint32_t m_euid;
   uint32_t m_egid;
   lldb::pid_t m_parent_pid;
@@ -216,4 +217,14 @@ class ProcessInstanceInfoMatch {
 
 } // namespace lldb_private
 
+LLVM_YAML_IS_SEQUENCE_VECTOR(lldb_private::ProcessInstanceInfo)
+
+namespace llvm {
+namespace yaml {
+template <> struct MappingTraits {
+  static void mapping(IO , lldb_private::ProcessInstanceInfo );
+};
+} // namespace yaml
+} // namespace llvm
+
 #endif // LLDB_UTILITY_PROCESSINFO_H

diff  --git a/lldb/source/Utility/ArchSpec.cpp 
b/lldb/source/Utility/ArchSpec.cpp
index bb4771c6488c..ca1ce4b3d378 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -1467,3 +1467,15 @@ void ArchSpec::DumpTriple(llvm::raw_ostream ) const {
   if (!environ_str.empty())
 s << "-" << environ_str;
 }
+
+void llvm::yaml::ScalarTraits::output(const ArchSpec , void *,
+raw_ostream ) {
+  Val.DumpTriple(Out);
+}
+
+llvm::StringRef
+llvm::yaml::ScalarTraits::input(llvm::StringRef Scalar, void *,
+  ArchSpec ) {
+  Val = ArchSpec(Scalar);
+  return {};
+}

diff  --git a/lldb/source/Utility/ProcessInfo.cpp 
b/lldb/source/Utility/ProcessInfo.cpp
index c78e62c394ff..450e62d8c5d6 100644
--- a/lldb/source/Utility/ProcessInfo.cpp
+++ b/lldb/source/Utility/ProcessInfo.cpp
@@ -331,3 +331,16 @@ void ProcessInstanceInfoMatch::Clear() {
   m_name_match_type = NameMatch::Ignore;
   m_match_all_users = false;
 }
+
+void llvm::yaml::MappingTraits::mapping(
+IO , ProcessInstanceInfo ) {
+  io.mapRequired("executable", Info.m_executable);
+  io.mapRequired("arg0", Info.m_arg0);
+  io.mapRequired("arch", Info.m_arch);
+  io.mapRequired("uid", Info.m_uid);
+  io.mapRequired("gid", Info.m_gid);
+  io.mapRequired("pid", Info.m_pid);
+  io.mapRequired("effective-uid", Info.m_euid);
+  io.mapRequired("effective-gid", Info.m_egid);
+  io.mapRequired("parent-pid", Info.m_parent_pid);
+}

diff  --git 

[Lldb-commits] [lldb] 638b06c - [lldb/Utility] Replace ProcessInstanceInfoList with std::vector. (NFCI)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-03-12T14:10:25-07:00
New Revision: 638b06cf298bc622c3ffd93dc4715c6f806de5b5

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

LOG: [lldb/Utility] Replace ProcessInstanceInfoList with std::vector. (NFCI)

Replace ProcessInstanceInfoList with std::vector
and update the call sites.

Added: 


Modified: 
lldb/include/lldb/Host/Host.h
lldb/include/lldb/Target/Platform.h
lldb/include/lldb/Utility/ProcessInfo.h
lldb/include/lldb/lldb-forward.h
lldb/source/Commands/CommandObjectPlatform.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Host/freebsd/Host.cpp
lldb/source/Host/linux/Host.cpp
lldb/source/Host/macosx/objcxx/Host.mm
lldb/source/Host/netbsd/Host.cpp
lldb/source/Host/openbsd/Host.cpp
lldb/source/Host/windows/Host.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformiOSSimulator.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/source/Target/Process.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/Host.h b/lldb/include/lldb/Host/Host.h
index 884c5cf63213..269bb18571b0 100644
--- a/lldb/include/lldb/Host/Host.h
+++ b/lldb/include/lldb/Host/Host.h
@@ -27,8 +27,8 @@ namespace lldb_private {
 class FileAction;
 class ProcessLaunchInfo;
 class ProcessInstanceInfo;
-class ProcessInstanceInfoList;
 class ProcessInstanceInfoMatch;
+typedef std::vector ProcessInstanceInfoList;
 
 // Exit Type for inferior processes
 struct WaitStatus {

diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index 7c2746ddec68..06290b34ac0e 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -33,8 +33,8 @@
 namespace lldb_private {
 
 class ProcessInstanceInfo;
-class ProcessInstanceInfoList;
 class ProcessInstanceInfoMatch;
+typedef std::vector ProcessInstanceInfoList;
 
 class ModuleCache;
 enum MmapFlags { eMmapFlagsPrivate = 1, eMmapFlagsAnon = 2 };

diff  --git a/lldb/include/lldb/Utility/ProcessInfo.h 
b/lldb/include/lldb/Utility/ProcessInfo.h
index c00b41c3ffd5..d2ad40487224 100644
--- a/lldb/include/lldb/Utility/ProcessInfo.h
+++ b/lldb/include/lldb/Utility/ProcessInfo.h
@@ -155,41 +155,7 @@ class ProcessInstanceInfo : public ProcessInfo {
   lldb::pid_t m_parent_pid;
 };
 
-class ProcessInstanceInfoList {
-public:
-  ProcessInstanceInfoList() = default;
-
-  void Clear() { m_infos.clear(); }
-
-  size_t GetSize() { return m_infos.size(); }
-
-  void Append(const ProcessInstanceInfo ) { m_infos.push_back(info); }
-
-  llvm::StringRef GetProcessNameAtIndex(size_t idx) {
-return ((idx < m_infos.size()) ? m_infos[idx].GetNameAsStringRef() : "");
-  }
-
-  lldb::pid_t GetProcessIDAtIndex(size_t idx) {
-return ((idx < m_infos.size()) ? m_infos[idx].GetProcessID() : 0);
-  }
-
-  bool GetInfoAtIndex(size_t idx, ProcessInstanceInfo ) {
-if (idx < m_infos.size()) {
-  info = m_infos[idx];
-  return true;
-}
-return false;
-  }
-
-  // You must ensure "idx" is valid before calling this function
-  const ProcessInstanceInfo (size_t idx) const {
-assert(idx < m_infos.size());
-return m_infos[idx];
-  }
-
-protected:
-  std::vector m_infos;
-};
+typedef std::vector ProcessInstanceInfoList;
 
 // ProcessInstanceInfoMatch
 //

diff  --git a/lldb/include/lldb/lldb-forward.h 
b/lldb/include/lldb/lldb-forward.h
index faca8fac62ef..6b22e50a553d 100644
--- a/lldb/include/lldb/lldb-forward.h
+++ b/lldb/include/lldb/lldb-forward.h
@@ -156,7 +156,6 @@ class Process;
 class ProcessAttachInfo;
 class ProcessInfo;
 class ProcessInstanceInfo;
-class ProcessInstanceInfoList;
 class ProcessInstanceInfoMatch;
 class ProcessLaunchInfo;
 class ProcessModID;

diff  --git a/lldb/source/Commands/CommandObjectPlatform.cpp 
b/lldb/source/Commands/CommandObjectPlatform.cpp
index e473756f1e93..5a6573307c61 100644
--- a/lldb/source/Commands/CommandObjectPlatform.cpp
+++ b/lldb/source/Commands/CommandObjectPlatform.cpp
@@ -1128,7 +1128,7 @@ class CommandObjectPlatformProcessList : public 
CommandObjectParsed {
   ProcessInstanceInfo::DumpTableHeader(ostrm, m_options.show_args,
m_options.verbose);
   for (uint32_t i = 0; i < matches; ++i) {
-proc_infos.GetProcessInfoAtIndex(i).DumpAsTableRow(
+proc_infos[i].DumpAsTableRow(
 ostrm, platform_sp->GetUserIDResolver(),
 

[Lldb-commits] [PATCH] D75537: Clear all settings during a test's setUp

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added a comment.

`platform.module-cache-directory` should be fixed by rGfe74df01a909 
.

Regarding `script-lang`, its default value is `eScriptLanguagePython` in 
CoreProperties.td and I'm not sure that I should change it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75537



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

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

[Jim Ingham via Phabricator via lldb-commits]

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

Removed Update till the next patch, and fixed a bug in PushPlan that preparing 
the next patch uncovered.




Comment at: lldb/include/lldb/Target/ThreadPlanStack.h:126-130
+auto result = m_plans_list.find(tid);
+if (result == m_plans_list.end())
+  return false;
+result->second.ThreadDestroyed(nullptr);
+m_plans_list.erase(result);

labath wrote:
> Any chance of calling `ThreadDestroyed` from ThreadPlanStack destructor, so 
> this can just be `m_plan_list.erase(tid)` ?
I dislike postponing things that might do needed work (like remove tell all the 
plans in the stack to remove their breakpoints) till a destructor happens to 
run.  That makes lldb's state harder to reason about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75880



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

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

[Jim Ingham via Phabricator via lldb-commits]

jingham updated this revision to Diff 250025.
jingham marked 6 inline comments as done.
jingham added a comment.

Removed ThreadPlanStackMap::Update, fixed a bug in PushPlan (can't std::move 
the ThreadPlan, THEN log it...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75880

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Thread.h
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Target/ThreadPlanStack.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Process.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadList.cpp
  lldb/source/Target/ThreadPlan.cpp
  lldb/source/Target/ThreadPlanStack.cpp

Index: lldb/source/Target/ThreadPlanStack.cpp
===
--- /dev/null
+++ lldb/source/Target/ThreadPlanStack.cpp
@@ -0,0 +1,370 @@
+//===-- ThreadPlanStack.cpp -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Target/ThreadPlanStack.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
+#include "lldb/Target/ThreadPlan.h"
+#include "lldb/Utility/Log.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+static void PrintPlanElement(Stream *s, const ThreadPlanSP ,
+ lldb::DescriptionLevel desc_level,
+ int32_t elem_idx) {
+  s->IndentMore();
+  s->Indent();
+  s->Printf("Element %d: ", elem_idx);
+  plan->GetDescription(s, desc_level);
+  s->EOL();
+  s->IndentLess();
+}
+
+void ThreadPlanStack::DumpThreadPlans(Stream *s,
+  lldb::DescriptionLevel desc_level,
+  bool include_internal) const {
+
+  uint32_t stack_size;
+
+  s->IndentMore();
+  s->Indent();
+  s->Printf("Active plan stack:\n");
+  int32_t print_idx = 0;
+  for (auto plan : m_plans) {
+PrintPlanElement(s, plan, desc_level, print_idx++);
+  }
+
+  if (AnyCompletedPlans()) {
+print_idx = 0;
+s->Indent();
+s->Printf("Completed Plan Stack:\n");
+for (auto plan : m_completed_plans)
+  PrintPlanElement(s, plan, desc_level, print_idx++);
+  }
+
+  if (AnyDiscardedPlans()) {
+print_idx = 0;
+s->Indent();
+s->Printf("Discarded Plan Stack:\n");
+for (auto plan : m_discarded_plans)
+  PrintPlanElement(s, plan, desc_level, print_idx++);
+  }
+
+  s->IndentLess();
+}
+
+size_t ThreadPlanStack::CheckpointCompletedPlans() {
+  m_completed_plan_checkpoint++;
+  m_completed_plan_store.insert(
+  std::make_pair(m_completed_plan_checkpoint, m_completed_plans));
+  return m_completed_plan_checkpoint;
+}
+
+void ThreadPlanStack::RestoreCompletedPlanCheckpoint(size_t checkpoint) {
+  auto result = m_completed_plan_store.find(checkpoint);
+  assert(result != m_completed_plan_store.end() &&
+ "Asked for a checkpoint that didn't exist");
+  m_completed_plans.swap((*result).second);
+  m_completed_plan_store.erase(result);
+}
+
+void ThreadPlanStack::DiscardCompletedPlanCheckpoint(size_t checkpoint) {
+  m_completed_plan_store.erase(checkpoint);
+}
+
+void ThreadPlanStack::ThreadDestroyed(Thread *thread) {
+  // Tell the plan stacks that this thread is going away:
+  for (ThreadPlanSP plan : m_plans)
+plan->ThreadDestroyed();
+
+  for (ThreadPlanSP plan : m_discarded_plans)
+plan->ThreadDestroyed();
+
+  for (ThreadPlanSP plan : m_completed_plans)
+plan->ThreadDestroyed();
+
+  // Now clear the current plan stacks:
+  m_plans.clear();
+  m_discarded_plans.clear();
+  m_completed_plans.clear();
+
+  // Push a ThreadPlanNull on the plan stack.  That way we can continue
+  // assuming that the plan stack is never empty, but if somebody errantly asks
+  // questions of a destroyed thread without checking first whether it is
+  // destroyed, they won't crash.
+  if (thread != nullptr) {
+lldb::ThreadPlanSP null_plan_sp(new ThreadPlanNull(*thread));
+m_plans.push_back(null_plan_sp);
+  }
+}
+
+void ThreadPlanStack::EnableTracer(bool value, bool single_stepping) {
+  for (ThreadPlanSP plan : m_plans) {
+if (plan->GetThreadPlanTracer()) {
+  plan->GetThreadPlanTracer()->EnableTracing(value);
+  plan->GetThreadPlanTracer()->EnableSingleStep(single_stepping);
+}
+  }
+}
+
+void ThreadPlanStack::SetTracer(lldb::ThreadPlanTracerSP _sp) {
+  for (ThreadPlanSP plan : m_plans)
+plan->SetThreadPlanTracer(tracer_sp);
+}
+
+void ThreadPlanStack::PushPlan(lldb::ThreadPlanSP new_plan_sp) {
+  // If the thread plan doesn't already have a tracer, give it its parent's
+  // tracer:
+  // The first plan has to be a base 

[Lldb-commits] [PATCH] D76080: Adjust error_msg handling for expect_expr in lldbtest.py

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 250021.
shafik added a comment.

Incorporate feedback on how to verify the results.


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

https://reviews.llvm.org/D76080

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


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2421,8 +2421,9 @@
 eval_result = frame.EvaluateExpression(expr, options)
 
 if error_msg:
-self.assertFalse(eval_result.IsValid(), "Unexpected success with 
result: '" + str(eval_result) + "'")
-self.assertEqual(error_msg, eval_result.GetError().GetCString())
+self.assertTrue(eval_result.IsValid(), "Result is not valid")
+self.assertTrue(eval_result.GetError().Fail(), "Unexpected 
success...")
+
self.assertTrue(eval_result.GetError().GetCString().find(error_msg) != -1)
 return
 
 if not eval_result.GetError().Success():


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2421,8 +2421,9 @@
 eval_result = frame.EvaluateExpression(expr, options)
 
 if error_msg:
-self.assertFalse(eval_result.IsValid(), "Unexpected success with result: '" + str(eval_result) + "'")
-self.assertEqual(error_msg, eval_result.GetError().GetCString())
+self.assertTrue(eval_result.IsValid(), "Result is not valid")
+self.assertTrue(eval_result.GetError().Fail(), "Unexpected success...")
+self.assertTrue(eval_result.GetError().GetCString().find(error_msg) != -1)
 return
 
 if not eval_result.GetError().Success():
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76080: Adjust error_msg handling for expect_expr in lldbtest.py

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

In D76080#1919862 , @jingham wrote:

>




> I'm on the fence about using a "find" not a strict string compare.  The only 
> reason you'd be passing in an error_msg is that you want to test that you got 
> the error string you were expecting.  I worry that using substrings will lead 
> to weakening this test by having too general a match.  OTOH, it would be 
> super annoying to match all of some of the compiler's error messages...

I really would prefer not to have to match string like this `warning: :1:4: '<=>' is a single token in C++20; add a space to avoid a 
change in behavior.*" != "warning: :1:4: '<=>' is a single 
token in C++20; add a space to avoid a change in behavior\nb1 <=> b2\n   
^\nerror: :1:6: expected expression\nb1 <=> b2\n ^\n"`

it also feels fragile to changes in formatting etc...


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

https://reviews.llvm.org/D76080



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

[Lldb-commits] [lldb] fe74df0 - [lldb] Specify default value for platform.module-cache-directory

[Tatyana Krasnukha via lldb-commits]

Author: Tatyana Krasnukha
Date: 2020-03-12T22:08:12+03:00
New Revision: fe74df01a909fb02528e83e90124f1b706176ddd

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

LOG: [lldb] Specify default value for platform.module-cache-directory

In addition to the commit rG352f16db87f583ec7f55f8028647b5fd8616111f,
this one fixes settings behavior on clearing - the setting should be
reverted to their default value, not an empty one.

Added: 


Modified: 
lldb/include/lldb/Target/Platform.h
lldb/source/Target/Platform.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index f347e7beae28..7c2746ddec68 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -50,6 +50,9 @@ class PlatformProperties : public Properties {
 
   FileSpec GetModuleCacheDirectory() const;
   bool SetModuleCacheDirectory(const FileSpec _spec);
+
+private:
+  void SetDefaultModuleCacheDirectory(const FileSpec _spec);
 };
 
 typedef std::shared_ptr PlatformPropertiesSP;

diff  --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index 3739ccd7edc6..eaa71b9cbbd0 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -26,6 +26,7 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/OptionParser.h"
+#include "lldb/Interpreter/OptionValueFileSpec.h"
 #include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Interpreter/Property.h"
 #include "lldb/Symbol/ObjectFile.h"
@@ -93,6 +94,7 @@ PlatformProperties::PlatformProperties() {
   module_cache_dir = FileSpec(user_home_dir.c_str());
   module_cache_dir.AppendPathComponent(".lldb");
   module_cache_dir.AppendPathComponent("module_cache");
+  SetDefaultModuleCacheDirectory(module_cache_dir);
   SetModuleCacheDirectory(module_cache_dir);
 }
 
@@ -117,6 +119,14 @@ bool PlatformProperties::SetModuleCacheDirectory(const 
FileSpec _spec) {
   nullptr, ePropertyModuleCacheDirectory, dir_spec);
 }
 
+void PlatformProperties::SetDefaultModuleCacheDirectory(
+const FileSpec _spec) {
+  auto f_spec_opt = m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpec(
+nullptr, false, ePropertyModuleCacheDirectory);
+  assert(f_spec_opt);
+  f_spec_opt->SetDefaultValue(dir_spec);
+}
+
 /// Get the native host platform plug-in.
 ///
 /// There should only be one of these for each host that LLDB runs



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

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

The original intention of this is to have any working environment, as quite 
often complex programs require many environment variables that are common to 
most processes. Having the user specify each of those is a bit too much to ask 
for, and they are complaining because of this. That said, just using the 
platform's environment would be enough to have something working. And it the 
extreme case when other specific env vars are needed, then the user can specify 
them by hand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74636



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

[Lldb-commits] [PATCH] D76080: Adjust error_msg handling for expect_expr in lldbtest.py

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

The result from EvaluateExpression is pretty much always going to be valid, 
since the result holds the error.  The correct way to do the first check is:

self.assertTrue(eval_result.GetError().Fail(), "Unexpected success...")

though you probably also want to make sure "eval_result.IsValid()" too, just to 
be on the safe side.  It's good to check that first, especially if you are 
going to do a find rather than a strict compare.  I don't think there's any 
guarantee that you have to leave the string in an Error empty if you return 
`False` from SBError().Fail().

I'm on the fence about using a "find" not a strict string compare.  The only 
reason you'd be passing in an error_msg is that you want to test that you got 
the error string you were expecting.  I worry that using substrings will lead 
to weakening this test by having too general a match.  OTOH, it would be super 
annoying to match all of some of the compiler's error messages...


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

https://reviews.llvm.org/D76080



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

[Lldb-commits] [PATCH] D76002: [lldb] Add YAML traits for ConstString and FileSpec

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbc9b6b33a0d5: [lldb/Utility] Add YAML traits for ConstString 
and FileSpec. (authored by JDevlieghere).

Changed prior to commit:
  https://reviews.llvm.org/D76002?vs=249650=249986#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76002

Files:
  lldb/include/lldb/Utility/ConstString.h
  lldb/include/lldb/Utility/FileSpec.h
  lldb/source/Utility/ConstString.cpp
  lldb/source/Utility/FileSpec.cpp
  lldb/unittests/Utility/ConstStringTest.cpp
  lldb/unittests/Utility/FileSpecTest.cpp

Index: lldb/unittests/Utility/FileSpecTest.cpp
===
--- lldb/unittests/Utility/FileSpecTest.cpp
+++ lldb/unittests/Utility/FileSpecTest.cpp
@@ -420,3 +420,24 @@
   EXPECT_TRUE(Match("", ""));
 
 }
+
+TEST(FileSpecTest, Yaml) {
+  std::string buffer;
+  llvm::raw_string_ostream os(buffer);
+
+  // Serialize.
+  FileSpec fs_windows("F:\\bar", FileSpec::Style::windows);
+  llvm::yaml::Output yout(os);
+  yout << fs_windows;
+  os.flush();
+
+  // Deserialize.
+  FileSpec deserialized;
+  llvm::yaml::Input yin(buffer);
+  yin >> deserialized;
+
+  EXPECT_EQ(deserialized.GetPathStyle(), fs_windows.GetPathStyle());
+  EXPECT_EQ(deserialized.GetFilename(), fs_windows.GetFilename());
+  EXPECT_EQ(deserialized.GetDirectory(), fs_windows.GetDirectory());
+  EXPECT_EQ(deserialized, fs_windows);
+}
Index: lldb/unittests/Utility/ConstStringTest.cpp
===
--- lldb/unittests/Utility/ConstStringTest.cpp
+++ lldb/unittests/Utility/ConstStringTest.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Utility/ConstString.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/YAMLParser.h"
 #include "gtest/gtest.h"
 
 using namespace lldb_private;
@@ -137,3 +138,22 @@
   EXPECT_TRUE(null == static_cast(nullptr));
   EXPECT_TRUE(null != "bar");
 }
+
+TEST(ConstStringTest, YAML) {
+  std::string buffer;
+  llvm::raw_string_ostream os(buffer);
+
+  // Serialize.
+  std::vector strings = {ConstString("foo"), ConstString("bar"),
+  ConstString("")};
+  llvm::yaml::Output yout(os);
+  yout << strings;
+  os.flush();
+
+  // Deserialize.
+  std::vector deserialized;
+  llvm::yaml::Input yin(buffer);
+  yin >> deserialized;
+
+  EXPECT_EQ(strings, deserialized);
+}
Index: lldb/source/Utility/FileSpec.cpp
===
--- lldb/source/Utility/FileSpec.cpp
+++ lldb/source/Utility/FileSpec.cpp
@@ -537,3 +537,19 @@
   if (!file.empty())
 Stream << file;
 }
+
+void llvm::yaml::ScalarEnumerationTraits::enumeration(
+IO , FileSpecStyle ) {
+  io.enumCase(value, "windows", FileSpecStyle(FileSpec::Style::windows));
+  io.enumCase(value, "posix", FileSpecStyle(FileSpec::Style::posix));
+  io.enumCase(value, "native", FileSpecStyle(FileSpec::Style::native));
+}
+
+void llvm::yaml::MappingTraits::mapping(IO , FileSpec ) {
+  io.mapRequired("directory", f.m_directory);
+  io.mapRequired("file", f.m_filename);
+  io.mapRequired("resolved", f.m_is_resolved);
+  FileSpecStyle style = f.m_style;
+  io.mapRequired("style", style);
+  f.m_style = style;
+}
Index: lldb/source/Utility/ConstString.cpp
===
--- lldb/source/Utility/ConstString.cpp
+++ lldb/source/Utility/ConstString.cpp
@@ -337,3 +337,15 @@
 llvm::StringRef Options) {
   format_provider::format(CS.GetStringRef(), OS, Options);
 }
+
+void llvm::yaml::ScalarTraits::output(const ConstString ,
+   void *, raw_ostream ) {
+  Out << Val.GetStringRef();
+}
+
+llvm::StringRef
+llvm::yaml::ScalarTraits::input(llvm::StringRef Scalar, void *,
+ ConstString ) {
+  Val = ConstString(Scalar);
+  return {};
+}
Index: lldb/include/lldb/Utility/FileSpec.h
===
--- lldb/include/lldb/Utility/FileSpec.h
+++ lldb/include/lldb/Utility/FileSpec.h
@@ -18,6 +18,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/YAMLTraits.h"
 
 #include 
 #include 
@@ -397,6 +398,8 @@
   ConstString GetLastPathComponent() const;
 
 protected:
+  friend struct llvm::yaml::MappingTraits;
+
   // Convenience method for setting the file without changing the style.
   void SetFile(llvm::StringRef path);
 
@@ -410,6 +413,8 @@
 /// Dump a FileSpec object to a stream
 Stream <<(Stream , const FileSpec );
 
+/// Prevent ODR violations with traits for llvm::sys::path::Style.
+LLVM_YAML_STRONG_TYPEDEF(FileSpec::Style, FileSpecStyle)
 } // namespace lldb_private
 
 namespace llvm {
@@ -436,6 +441,16 @@
   static void 

[Lldb-commits] [PATCH] D76080: Adjust error_msg handling for expect_expr in lldbtest.py

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: teemperor, labath, aprantl.
shafik added a comment.

I am open to suggestions on alternative approaches, for some context I ran into 
this trying to add a failing test to D75761  
as was suggested.


I was trying to use the `error_msg` argument for `expect_expr` (looks like I am 
the first one) and the assumption that `eval_result.IsValid()` is false does 
not look correct, so I removed it.

I also changed the way we check of the error messages matches from an exact 
match to just looking for the string withing the error message. Matching the 
whole error message does not feel necessary.


https://reviews.llvm.org/D76080

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


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2421,8 +2421,7 @@
 eval_result = frame.EvaluateExpression(expr, options)
 
 if error_msg:
-self.assertFalse(eval_result.IsValid(), "Unexpected success with 
result: '" + str(eval_result) + "'")
-self.assertEqual(error_msg, eval_result.GetError().GetCString())
+
self.assertTrue(eval_result.GetError().GetCString().find(error_msg) != -1)
 return
 
 if not eval_result.GetError().Success():


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2421,8 +2421,7 @@
 eval_result = frame.EvaluateExpression(expr, options)
 
 if error_msg:
-self.assertFalse(eval_result.IsValid(), "Unexpected success with result: '" + str(eval_result) + "'")
-self.assertEqual(error_msg, eval_result.GetError().GetCString())
+self.assertTrue(eval_result.GetError().GetCString().find(error_msg) != -1)
 return
 
 if not eval_result.GetError().Success():
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76009: [lldb/Target] Initialize new targets environment variables from target.env-vars

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

BTW, don't worry about the `OS_ACTIVITY_DT_MODE` env var.  That's something you 
have to set when debugging or the Foundation loggging method (NSLog) will go to 
the system console not to the current terminal, so we force it on all processes 
on macOS...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76009



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

[Lldb-commits] [PATCH] D76080: Adjust error_msg handling for expect_expr in lldbtest.py

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

I am open to suggestions on alternative approaches, for some context I ran into 
this trying to add a failing test to D75761  
as was suggested.


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

https://reviews.llvm.org/D76080



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

[Lldb-commits] [PATCH] D76009: [lldb/Target] Initialize new targets environment variables from target.env-vars

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D76009#1919554 , @friss wrote:

> In D76009#1919103 , @labath wrote:
>
> > Actually, hang on.
> >
> > > One existing test had to be modified, because the initialization of
> > >  the environment properties now take place at the time the target is
> > >  created, not at the first use of the environment (usually launch
> > >  time).
>
>
> Just to be clear, in that test the host environment was modified between the 
> creation of the target and the launch which I think is pretty unlikely to 
> matter in practice.
>
> > Does this mean that the target-local value of the `target.inherit-env` 
> > setting no longer has any effect? Currently I can set it after creating a 
> > target (but before launching) to stop the process inheriting the default 
> > environment:
> > 
> >   (lldb) file /usr/bin/env
> >   Current executable set to '/usr/bin/env' (x86_64).
> >   (lldb) set set target.inherit-env false
> >   (lldb) pr la
> >   Process 26684 launched: '/usr/bin/env' (x86_64)
> >   Process 26684 exited with status = 0 (0x) 
> > 
> > 
> > I take it that after this, that will no longer be possible? It would still 
> > be possible to do that by setting the global value of the setting before 
> > creating a target, but the target-local setting would be useless (?)
>
> The inherited environment is collected the first time the property is 
> accessed and running the callback triggers this. For some reason just doing 
> `set show target.env-vars` doesn't trigger it though, I'd need to debug this 
> to understand. So yes, with this change, the `target.inherit-env` setting 
> will take effect at target creation time and not at whatever next point in 
> time the lazy logic is triggered.
>
> One way to fix this is to add another callback for the `target.inherit-env` 
> property and remove the variables that are in the environment.
>
> > I'm not really sure how these settings are supposed to work, but this does 
> > not seem ideal to me.
>
> Jim would certainly be able to explain the intent here.


I don't think the current version of the code works very well.  You can 
actually only change the value of target.inherit-env BEFORE the first run.  For 
instance:

   > lldb printenv
  warning: Overwriting existing definition for 'p'.
  warning: Overwriting existing definition for 't'.
  (lldb) target create "printenv"
  Current executable set to 'printenv' (x86_64).
  (lldb) set set target.in
  Available completions:
target.inherit-env   
target.input-path
target.inline-breakpoint-strategy
  (lldb) set set target.inherit-env 0
  (lldb) set show target.env-vars
  target.env-vars (dictionary of strings) =
  (lldb) run
  Process 65813 launched: '/tmp/printenv' (x86_64)
  0  :OS_ACTIVITY_DT_MODE=enable:
  Process 65813 exited with status = 0 (0x) 
  (lldb) set set target.inherit-env 1
  (lldb) run
  Process 65816 launched: '/tmp/printenv' (x86_64)
  0  :OS_ACTIVITY_DT_MODE=enable:
  Process 65816 exited with status = 0 (0x) 

lldb had way more environment variables than that...

And conversely, if you run with inherit-env set to 1 and then set it to 0, you 
still get all the environment variables.

For this to behave correctly, we would as Fred suggests have to add a callback 
to setting inherit-env and add or remove the host's variables as appropriate 
when it changes.

IMO there should really also be a "target create" option for inherit-env.   
This is usually something you want to do depending on the type of target, and 
you don't generally change it after the fact.  It's awkward to have to change a 
setting to create this or that target in a certain mode.  It would be better if 
we had predicates for the settings, then you could do something like `settings 
set target[platform=remote].inherit-env 0`.  But we haven't gotten there yet...

Note, for this to be really useful for remote debugging, lldb should be able to 
fetch the monitor's environment and use that to prime the target's environment 
when inherit-env is on.  That would be a much more useful thing to do.  But 
that's way outside the scope of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76009



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

[Lldb-commits] [lldb] bc9b6b3 - [lldb/Utility] Add YAML traits for ConstString and FileSpec.

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-03-12T10:28:21-07:00
New Revision: bc9b6b33a0d5760370e72ae06c520c25aa8d61c6

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

LOG: [lldb/Utility] Add YAML traits for ConstString and FileSpec.

Add YAML traits for the ConstString and FileSpec classes so they can be
serialized as part of ProcessInfo. The latter needs to be serializable
for the reproducers.

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

Added: 


Modified: 
lldb/include/lldb/Utility/ConstString.h
lldb/include/lldb/Utility/FileSpec.h
lldb/source/Utility/ConstString.cpp
lldb/source/Utility/FileSpec.cpp
lldb/unittests/Utility/ConstStringTest.cpp
lldb/unittests/Utility/FileSpecTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/ConstString.h 
b/lldb/include/lldb/Utility/ConstString.h
index ee605dad9493..c2419407f2f6 100644
--- a/lldb/include/lldb/Utility/ConstString.h
+++ b/lldb/include/lldb/Utility/ConstString.h
@@ -9,9 +9,10 @@
 #ifndef LLDB_UTILITY_CONSTSTRING_H
 #define LLDB_UTILITY_CONSTSTRING_H
 
-#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/YAMLTraits.h"
 
 #include 
 
@@ -481,6 +482,16 @@ template <> struct DenseMapInfo 
{
   }
 };
 /// \}
-}
+
+namespace yaml {
+template <> struct ScalarTraits {
+  static void output(const lldb_private::ConstString &, void *, raw_ostream &);
+  static StringRef input(StringRef, void *, lldb_private::ConstString &);
+  static QuotingType mustQuote(StringRef S) { return QuotingType::Double; }
+};
+} // namespace yaml
+} // namespace llvm
+
+LLVM_YAML_IS_SEQUENCE_VECTOR(lldb_private::ConstString)
 
 #endif // LLDB_UTILITY_CONSTSTRING_H

diff  --git a/lldb/include/lldb/Utility/FileSpec.h 
b/lldb/include/lldb/Utility/FileSpec.h
index 119b33d54d4e..f7cbeb247100 100644
--- a/lldb/include/lldb/Utility/FileSpec.h
+++ b/lldb/include/lldb/Utility/FileSpec.h
@@ -18,6 +18,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/YAMLTraits.h"
 
 #include 
 #include 
@@ -397,6 +398,8 @@ class FileSpec {
   ConstString GetLastPathComponent() const;
 
 protected:
+  friend struct llvm::yaml::MappingTraits;
+
   // Convenience method for setting the file without changing the style.
   void SetFile(llvm::StringRef path);
 
@@ -410,6 +413,8 @@ class FileSpec {
 /// Dump a FileSpec object to a stream
 Stream <<(Stream , const FileSpec );
 
+/// Prevent ODR violations with traits for llvm::sys::path::Style.
+LLVM_YAML_STRONG_TYPEDEF(FileSpec::Style, FileSpecStyle)
 } // namespace lldb_private
 
 namespace llvm {
@@ -436,6 +441,16 @@ template <> struct format_provider 
{
   static void format(const lldb_private::FileSpec , llvm::raw_ostream 
,
  StringRef Style);
 };
+
+namespace yaml {
+template <> struct ScalarEnumerationTraits {
+  static void enumeration(IO , lldb_private::FileSpecStyle );
+};
+
+template <> struct MappingTraits {
+  static void mapping(IO , lldb_private::FileSpec );
+};
+} // namespace yaml
 } // namespace llvm
 
 #endif // LLDB_UTILITY_FILESPEC_H

diff  --git a/lldb/source/Utility/ConstString.cpp 
b/lldb/source/Utility/ConstString.cpp
index 8911a06218bc..62f79b3df7a5 100644
--- a/lldb/source/Utility/ConstString.cpp
+++ b/lldb/source/Utility/ConstString.cpp
@@ -337,3 +337,15 @@ void llvm::format_provider::format(const 
ConstString ,
 llvm::StringRef Options) {
   format_provider::format(CS.GetStringRef(), OS, Options);
 }
+
+void llvm::yaml::ScalarTraits::output(const ConstString ,
+   void *, raw_ostream ) {
+  Out << Val.GetStringRef();
+}
+
+llvm::StringRef
+llvm::yaml::ScalarTraits::input(llvm::StringRef Scalar, void *,
+ ConstString ) {
+  Val = ConstString(Scalar);
+  return {};
+}

diff  --git a/lldb/source/Utility/FileSpec.cpp 
b/lldb/source/Utility/FileSpec.cpp
index f732b43f700e..1ec5d60e2780 100644
--- a/lldb/source/Utility/FileSpec.cpp
+++ b/lldb/source/Utility/FileSpec.cpp
@@ -537,3 +537,19 @@ void llvm::format_provider::format(const 
FileSpec ,
   if (!file.empty())
 Stream << file;
 }
+
+void llvm::yaml::ScalarEnumerationTraits::enumeration(
+IO , FileSpecStyle ) {
+  io.enumCase(value, "windows", FileSpecStyle(FileSpec::Style::windows));
+  io.enumCase(value, "posix", FileSpecStyle(FileSpec::Style::posix));
+  io.enumCase(value, "native", FileSpecStyle(FileSpec::Style::native));
+}
+
+void llvm::yaml::MappingTraits::mapping(IO , FileSpec ) {
+  io.mapRequired("directory", f.m_directory);
+  io.mapRequired("file", 

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[Hsiangkai Wang via Phabricator via lldb-commits]

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



Comment at: llvm/include/llvm/Support/ELFAttributes.h:32
+   bool HasTagPrefix = true);
+Optional attrTypeFromString(StringRef Tag, TagNameMap Map);
+

jhenderson wrote:
> HsiangKai wrote:
> > jhenderson wrote:
> > > Can this be `Optional`?
> > `AttrType` is the enum in `ELFAttrs`. There is also `AttrType` in 
> > `ARMBuildAttributes` and `RISCVAttributes`. They all could be implicitly 
> > converted to integer. So, I use integer as the common interface between 
> > different architecture attributes enum.
> Okay, seems reasonable. I wonder if AttrType needs to be specified as having 
> `unsigned` underlying tap (in all cases) to be consistent with the 
> `TagNameItem::attr` member.
OK, I will specify the underlying type explicitly.



Comment at: llvm/unittests/Support/ELFAttributeParser.cpp:1
+#include "llvm/Support/ELFAttributeParser.h"
+#include "llvm/Support/ARMAttributeParser.h"

jhenderson wrote:
> Missing licence header.
> 
> Test files usually are called *Test.cpp, where * is the base file/class that 
> is being tested. It seems like this file is only testing the 
> ARMAttributeParser. Should it be called "ARMAttributeParserTest.cpp"
This file tests the common part of the attribute section, i.e., attribute 
section header. However, since ELFAttributeParser is an abstract class, I need 
to use ARMAttributeParser or RISCVAttributeParser as the concrete object to run 
the test. I just pick ARMAttributeParser as the concrete object. This test is 
not used to test ARMAttributeParser.

You could refer to https://reviews.llvm.org/D74023#1911405. @MaskRay suggests 
to extract the common part out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

[David Stenberg via Phabricator via lldb-commits]

dstenb added a comment.

In D73534#1918940 , @dstenb wrote:

> In D73534#1918890 , @manojgupta 
> wrote:
>
> > Hi,
> >
> > I see another crash with this change when building gdb.
> >
> > Reduced test case:
> >  struct type *a(type *, type *, long, long);
> >  enum b {};
> >  static int empty_array(type *, int c) { type *d = a(__null, d, c, c - 1); }
> >  long e;
> >  b f() { empty_array(0, e); }
> >
> > Repros with:
> >  clang -cc1 -triple x86_64-linux-gnu -emit-obj -disable-free 
> > -mrelocation-model pic -pic-level 2 -pic-is-pie -mthread-model posix 
> > -mframe-pointer=all  -mconstructor-aliases -munwind-tables  
> > -dwarf-column-info  -debug-info-kind=limited -dwarf-version=4 
> > -debugger-tuning=gdb  -O2  -x c++
> >
> > Chromium bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1060788
>
>
> Oh, that assertion is related to D75036  
> which I did. I can have a look at that.


I wrote a PR for that: https://bugs.llvm.org/show_bug.cgi?id=45181.

I'll see if I'm able to put together something for that today.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D75929: [DebugInfo] Support DWARFv5 index sections.

[Igor Kudrin via Phabricator via lldb-commits]

ikudrin updated this revision to Diff 249939.
ikudrin added a comment.

- Use values for clashing identifiers proposed by @dblaikie.
- Convert all unknown section identifiers into a special value, 
`DW_SECT_EXT_unknown`; Use an optional parallel array to keep the raw values of 
unknown identifiers.
- Split the refactoring part in `llvm-dwp.cpp` into a separate patch, D76067 
.

There are some other changes that can be split into separate patches. I will 
make the series when the direction of this patch is approved in general.


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

https://reviews.llvm.org/D75929

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
  llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
  llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
  llvm/test/DebugInfo/X86/dwp-v2-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v2-loc.s
  llvm/test/DebugInfo/X86/dwp-v2-tu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-loclists.s
  llvm/test/DebugInfo/X86/dwp-v5-tu-index.s
  llvm/test/tools/llvm-dwp/X86/unsupported_cu_index_version.s
  llvm/tools/llvm-dwp/llvm-dwp.cpp

Index: llvm/tools/llvm-dwp/llvm-dwp.cpp
===
--- llvm/tools/llvm-dwp/llvm-dwp.cpp
+++ llvm/tools/llvm-dwp/llvm-dwp.cpp
@@ -214,11 +214,12 @@
   StringRef DWPName;
 };
 
-// Convert a section identifier into the index to use with
+// Convert an internal section identifier into the index to use with
 // UnitIndexEntry::Contributions.
 static unsigned SectionKindToIndex(DWARFSectionKind Kind) {
-  assert(static_cast(Kind) >= 1);
-  return static_cast(Kind) - 1;
+  // Assuming pre-standard DWP format.
+  assert(SerializeSectionKind(Kind, 2) >= 1);
+  return SerializeSectionKind(Kind, 2) - 1;
 }
 
 // Convert a UnitIndexEntry::Contributions index to the corresponding on-disk
@@ -250,12 +251,14 @@
 // Zero out the debug_info contribution
 Entry.Contributions[0] = {};
 for (auto Kind : TUIndex.getColumnKinds()) {
+  if (Kind == DW_SECT_EXT_unknown)
+continue;
   auto  = Entry.Contributions[SectionKindToIndex(Kind)];
   C.Offset += I->Offset;
   C.Length = I->Length;
   ++I;
 }
-const unsigned TypesIndex = SectionKindToIndex(DW_SECT_TYPES);
+const unsigned TypesIndex = SectionKindToIndex(DW_SECT_EXT_TYPES);
 auto  = Entry.Contributions[TypesIndex];
 Out.emitBytes(Types.substr(
 C.Offset - TUEntry.Contributions[TypesIndex].Offset, C.Length));
@@ -277,7 +280,7 @@
   UnitIndexEntry Entry = CUEntry;
   // Zero out the debug_info contribution
   Entry.Contributions[0] = {};
-  auto  = Entry.Contributions[SectionKindToIndex(DW_SECT_TYPES)];
+  auto  = Entry.Contributions[SectionKindToIndex(DW_SECT_EXT_TYPES)];
   C.Offset = TypesOffset;
   auto PrevOffset = Offset;
   // Length of the unit, including the 4 byte length field.
@@ -448,7 +451,7 @@
 
   if (DWARFSectionKind Kind = SectionPair->second.second) {
 auto Index = SectionKindToIndex(Kind);
-if (Kind != DW_SECT_TYPES) {
+if (Kind != DW_SECT_EXT_TYPES) {
   CurEntry.Contributions[Index].Offset = ContributionOffsets[Index];
   ContributionOffsets[Index] +=
   (CurEntry.Contributions[Index].Length = Contents.size());
@@ -532,10 +535,10 @@
   MCSection *const TUIndexSection = MCOFI.getDwarfTUIndexSection();
   const StringMap> KnownSections = {
   {"debug_info.dwo", {MCOFI.getDwarfInfoDWOSection(), DW_SECT_INFO}},
-  {"debug_types.dwo", {MCOFI.getDwarfTypesDWOSection(), DW_SECT_TYPES}},
+  {"debug_types.dwo", {MCOFI.getDwarfTypesDWOSection(), DW_SECT_EXT_TYPES}},
   {"debug_str_offsets.dwo", {StrOffsetSection, DW_SECT_STR_OFFSETS}},
   {"debug_str.dwo", {StrSection, static_cast(0)}},
-  {"debug_loc.dwo", {MCOFI.getDwarfLocDWOSection(), DW_SECT_LOC}},
+  {"debug_loc.dwo", {MCOFI.getDwarfLocDWOSection(), DW_SECT_EXT_LOC}},
   {"debug_line.dwo", {MCOFI.getDwarfLineDWOSection(), DW_SECT_LINE}},
   {"debug_abbrev.dwo", {MCOFI.getDwarfAbbrevDWOSection(), DW_SECT_ABBREV}},
   {"debug_cu_index", {CUIndexSection, static_cast(0)}},
@@ -599,7 +602,7 @@
   P.first->second.DWOName = ID.DWOName;
   addAllTypes(Out, TypeIndexEntries, TypesSection, CurTypesSection,
   CurEntry,
-  ContributionOffsets[SectionKindToIndex(DW_SECT_TYPES)]);
+  ContributionOffsets[SectionKindToIndex(DW_SECT_EXT_TYPES)]);
   continue;
 }
 
@@ -607,6 +610,8 @@
 DataExtractor CUIndexData(CurCUIndexSection, Obj.isLittleEndian(), 0);
 if (!CUIndex.parse(CUIndexData))
   return make_error("Failed to parse cu_index");
+if 

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

[David Stenberg via Phabricator via lldb-commits]

dstenb added a comment.

In D73534#1918890 , @manojgupta wrote:

> Hi,
>
> I see another crash with this change when building gdb.
>
> Reduced test case:
>  struct type *a(type *, type *, long, long);
>  enum b {};
>  static int empty_array(type *, int c) { type *d = a(__null, d, c, c - 1); }
>  long e;
>  b f() { empty_array(0, e); }
>
> Repros with:
>  clang -cc1 -triple x86_64-linux-gnu -emit-obj -disable-free 
> -mrelocation-model pic -pic-level 2 -pic-is-pie -mthread-model posix 
> -mframe-pointer=all  -mconstructor-aliases -munwind-tables  
> -dwarf-column-info  -debug-info-kind=limited -dwarf-version=4 
> -debugger-tuning=gdb  -O2  -x c++
>
> Chromium bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1060788


Oh, that assertion is related to D75036  which 
I did. I can have a look at that.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[Hsiangkai Wang via Phabricator via lldb-commits]

HsiangKai updated this revision to Diff 249854.
HsiangKai added a comment.

Fix unit test errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023

Files:
  lld/ELF/InputFiles.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  llvm/include/llvm/BinaryFormat/ELF.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/include/llvm/Support/ARMAttributeParser.h
  llvm/include/llvm/Support/ARMBuildAttributes.h
  llvm/include/llvm/Support/ELFAttributeParser.h
  llvm/include/llvm/Support/ELFAttributes.h
  llvm/include/llvm/Support/RISCVAttributeParser.h
  llvm/include/llvm/Support/RISCVAttributes.h
  llvm/lib/Object/ELF.cpp
  llvm/lib/Object/ELFObjectFile.cpp
  llvm/lib/ObjectYAML/ELFYAML.cpp
  llvm/lib/Support/ARMAttributeParser.cpp
  llvm/lib/Support/ARMBuildAttrs.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/ELFAttributeParser.cpp
  llvm/lib/Support/ELFAttributes.cpp
  llvm/lib/Support/RISCVAttributeParser.cpp
  llvm/lib/Support/RISCVAttributes.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/attribute-with-insts.s
  llvm/test/MC/RISCV/attribute.s
  llvm/test/MC/RISCV/invalid-attribute.s
  llvm/test/tools/llvm-objdump/RISCV/lit.local.cfg
  llvm/test/tools/llvm-objdump/RISCV/unknown-arch-attr.test
  llvm/tools/llvm-readobj/ELFDumper.cpp
  llvm/unittests/Support/ARMAttributeParser.cpp
  llvm/unittests/Support/CMakeLists.txt
  llvm/unittests/Support/ELFAttributeParserTest.cpp
  llvm/unittests/Support/RISCVAttributeParserTest.cpp

Index: llvm/unittests/Support/RISCVAttributeParserTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/RISCVAttributeParserTest.cpp
@@ -0,0 +1,69 @@
+//===- unittests/RISCVAttributeParserTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "llvm/Support/RISCVAttributeParser.h"
+#include "llvm/Support/ARMBuildAttributes.h"
+#include "llvm/Support/ELFAttributes.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace llvm;
+
+struct RISCVAttributeSection {
+  unsigned Tag;
+  unsigned Value;
+
+  RISCVAttributeSection(unsigned tag, unsigned value) : Tag(tag), Value(value) {}
+
+  void write(raw_ostream ) {
+OS.flush();
+// length = length + "riscv\0" + TagFile + ByteSize + Tag + Value;
+// length = 17 bytes
+
+OS << 'A' << (uint8_t)17 << (uint8_t)0 << (uint8_t)0 << (uint8_t)0;
+OS << "riscv" << '\0';
+OS << (uint8_t)1 << (uint8_t)7 << (uint8_t)0 << (uint8_t)0 << (uint8_t)0;
+OS << (uint8_t)Tag << (uint8_t)Value;
+  }
+};
+
+static bool testAttribute(unsigned Tag, unsigned Value, unsigned ExpectedTag,
+   unsigned ExpectedValue) {
+  std::string buffer;
+  raw_string_ostream OS(buffer);
+  RISCVAttributeSection Section(Tag, Value);
+  Section.write(OS);
+  ArrayRef Bytes(reinterpret_cast(OS.str().c_str()),
+  OS.str().size());
+
+  RISCVAttributeParser Parser;
+  cantFail(Parser.parse(Bytes, support::little));
+
+  Optional Attr = Parser.getAttributeValue(ExpectedTag);
+  return Attr.hasValue() && Attr.getValue() == ExpectedValue;
+}
+
+static bool testTagString(unsigned Tag, const char *name) {
+  return ELFAttrs::attrTypeAsString(Tag, RISCVAttrs::RISCVAttributeTags)
+ .str() == name;
+}
+
+TEST(StackAlign, testAttribute) {
+  EXPECT_TRUE(testTagString(4, "Tag_stack_align"));
+  EXPECT_TRUE(
+  testAttribute(4, 4, RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_4));
+  EXPECT_TRUE(
+  testAttribute(4, 16, RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_16));
+}
+
+TEST(UnalignedAccess, testAttribute) {
+  EXPECT_TRUE(testTagString(6, "Tag_unaligned_access"));
+  EXPECT_TRUE(testAttribute(6, 0, RISCVAttrs::UNALIGNED_ACCESS,
+RISCVAttrs::NOT_ALLOWED));
+  EXPECT_TRUE(
+  testAttribute(6, 1, RISCVAttrs::UNALIGNED_ACCESS, RISCVAttrs::ALLOWED));
+}
Index: llvm/unittests/Support/ELFAttributeParserTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/ELFAttributeParserTest.cpp
@@ -0,0 +1,47 @@
+//===- unittests/ELFAttributeParserTest.cpp ---===//
+//
+// Part 

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

[Manoj Gupta via Phabricator via lldb-commits]

manojgupta added a comment.

Hi,

I see another crash with this change when building gdb.

Reduced test case:
struct type *a(type *, type *, long, long);
enum b {};
static int empty_array(type *, int c) { type *d = a(__null, d, c, c - 1); }
long e;
b f() { empty_array(0, e); }

Repros with:
clang -cc1 -triple x86_64-linux-gnu -emit-obj -disable-free -mrelocation-model 
pic -pic-level 2 -pic-is-pie -mthread-model posix -mframe-pointer=all  
-mconstructor-aliases -munwind-tables  -dwarf-column-info  
-debug-info-kind=limited -dwarf-version=4 -debugger-tuning=gdb  -O2  -x c++

Chromium bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1060788


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D73534#1918818 , @djtodoro wrote:

> In D73534#1916309 , @djtodoro wrote:
>
> > In D73534#1916291 , @djtodoro 
> > wrote:
> >
> > > Thanks for reporting this! Since this is the case of the `X86::MOV16ri` 
> > > the D75326  will solve this issue.
> >
> >
> > The alternative is D75974 .
>
>
> The D75974  is commited.


Thanks!


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[Hsiangkai Wang via Phabricator via lldb-commits]

HsiangKai updated this revision to Diff 249845.
HsiangKai added a comment.

- Specify underlying type for AttrType.
- Update unit tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023

Files:
  lld/ELF/InputFiles.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  llvm/include/llvm/BinaryFormat/ELF.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/include/llvm/Support/ARMAttributeParser.h
  llvm/include/llvm/Support/ARMBuildAttributes.h
  llvm/include/llvm/Support/ELFAttributeParser.h
  llvm/include/llvm/Support/ELFAttributes.h
  llvm/include/llvm/Support/RISCVAttributeParser.h
  llvm/include/llvm/Support/RISCVAttributes.h
  llvm/lib/Object/ELF.cpp
  llvm/lib/Object/ELFObjectFile.cpp
  llvm/lib/ObjectYAML/ELFYAML.cpp
  llvm/lib/Support/ARMAttributeParser.cpp
  llvm/lib/Support/ARMBuildAttrs.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/ELFAttributeParser.cpp
  llvm/lib/Support/ELFAttributes.cpp
  llvm/lib/Support/RISCVAttributeParser.cpp
  llvm/lib/Support/RISCVAttributes.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/attribute-with-insts.s
  llvm/test/MC/RISCV/attribute.s
  llvm/test/MC/RISCV/invalid-attribute.s
  llvm/tools/llvm-readobj/ELFDumper.cpp
  llvm/unittests/Support/ARMAttributeParser.cpp
  llvm/unittests/Support/CMakeLists.txt
  llvm/unittests/Support/ELFAttributeParserTest.cpp
  llvm/unittests/Support/RISCVAttributeParserTest.cpp

Index: llvm/unittests/Support/RISCVAttributeParserTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/RISCVAttributeParserTest.cpp
@@ -0,0 +1,69 @@
+//===- unittests/RISCVAttributeParserTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "llvm/Support/RISCVAttributeParser.h"
+#include "llvm/Support/ARMBuildAttributes.h"
+#include "llvm/Support/ELFAttributes.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace llvm;
+
+struct AttributeSection {
+  unsigned Tag;
+  unsigned Value;
+
+  AttributeSection(unsigned tag, unsigned value) : Tag(tag), Value(value) {}
+
+  void write(raw_ostream ) {
+OS.flush();
+// length = length + "riscv\0" + TagFile + ByteSize + Tag + Value;
+// length = 17 bytes
+
+OS << 'A' << (uint8_t)17 << (uint8_t)0 << (uint8_t)0 << (uint8_t)0;
+OS << "riscv" << '\0';
+OS << (uint8_t)1 << (uint8_t)7 << (uint8_t)0 << (uint8_t)0 << (uint8_t)0;
+OS << (uint8_t)Tag << (uint8_t)Value;
+  }
+};
+
+bool testBuildAttr(unsigned Tag, unsigned Value, unsigned ExpectedTag,
+   unsigned ExpectedValue) {
+  std::string buffer;
+  raw_string_ostream OS(buffer);
+  AttributeSection Section(Tag, Value);
+  Section.write(OS);
+  ArrayRef Bytes(reinterpret_cast(OS.str().c_str()),
+  OS.str().size());
+
+  RISCVAttributeParser Parser;
+  cantFail(Parser.parse(Bytes, support::little));
+
+  Optional Attr = Parser.getAttributeValue(ExpectedTag);
+  return Attr.hasValue() && Attr.getValue() == ExpectedValue;
+}
+
+bool testTagString(unsigned Tag, const char *name) {
+  return ELFAttrs::attrTypeAsString(Tag, RISCVAttrs::RISCVAttributeTags)
+ .str() == name;
+}
+
+TEST(StackAlign, testBuildAttr) {
+  EXPECT_TRUE(testTagString(4, "Tag_stack_align"));
+  EXPECT_TRUE(
+  testBuildAttr(4, 4, RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_4));
+  EXPECT_TRUE(
+  testBuildAttr(4, 16, RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_16));
+}
+
+TEST(UnalignedAccess, testBuildAttr) {
+  EXPECT_TRUE(testTagString(6, "Tag_unaligned_access"));
+  EXPECT_TRUE(testBuildAttr(6, 0, RISCVAttrs::UNALIGNED_ACCESS,
+RISCVAttrs::NOT_ALLOWED));
+  EXPECT_TRUE(
+  testBuildAttr(6, 1, RISCVAttrs::UNALIGNED_ACCESS, RISCVAttrs::ALLOWED));
+}
Index: llvm/unittests/Support/ELFAttributeParserTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/ELFAttributeParserTest.cpp
@@ -0,0 +1,47 @@
+//===- unittests/ELFAttributeParserTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See 

[Lldb-commits] [PATCH] D76009: [lldb/Target] Initialize new targets environment variables from target.env-vars

[Frederic Riss via Phabricator via lldb-commits]

friss added a comment.

In D76009#1919103 , @labath wrote:

> Actually, hang on.
>
> > One existing test had to be modified, because the initialization of
> >  the environment properties now take place at the time the target is
> >  created, not at the first use of the environment (usually launch
> >  time).


Just to be clear, in that test the host environment was modified between the 
creation of the target and the launch which I think is pretty unlikely to 
matter in practice.

> Does this mean that the target-local value of the `target.inherit-env` 
> setting no longer has any effect? Currently I can set it after creating a 
> target (but before launching) to stop the process inheriting the default 
> environment:
> 
>   (lldb) file /usr/bin/env
>   Current executable set to '/usr/bin/env' (x86_64).
>   (lldb) set set target.inherit-env false
>   (lldb) pr la
>   Process 26684 launched: '/usr/bin/env' (x86_64)
>   Process 26684 exited with status = 0 (0x) 
> 
> 
> I take it that after this, that will no longer be possible? It would still be 
> possible to do that by setting the global value of the setting before 
> creating a target, but the target-local setting would be useless (?)

The inherited environment is collected the first time the property is accessed 
and running the callback triggers this. For some reason just doing `set show 
target.env-vars` doesn't trigger it though, I'd need to debug this to 
understand. So yes, with this change, the `target.inherit-env` setting will 
take effect at target creation time and not at whatever next point in time the 
lazy logic is triggered.

One way to fix this is to add another callback for the `target.inherit-env` 
property and remove the variables that are in the environment.

> I'm not really sure how these settings are supposed to work, but this does 
> not seem ideal to me.

Jim would certainly be able to explain the intent here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76009



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

[Lldb-commits] [PATCH] D75537: Clear all settings during a test's setUp

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

Fixed in 352f16db87f583ec7f55f8028647b5fd8616111f 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75537



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

[Lldb-commits] [lldb] 352f16d - [lldb] Let OptionValueRegex::Clear set to value to the default and not an empty regex

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-03-12T16:12:14+01:00
New Revision: 352f16db87f583ec7f55f8028647b5fd8616111f

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

LOG: [lldb] Let OptionValueRegex::Clear set to value to the default and not an 
empty regex

Since D75537 the test suite clears all settings before a test. This caused
two tests to fail:
lldb-api :: functionalities/inline-stepping/TestInlineStepping.py
lldb-api :: 
lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
The reason for that is that OptionValueRegex::Clear was setting the regex
to empty instead of the default value that was passed initially. This caused
that the target.process.thread.step-avoid-regexp setting which is used in the
tests was set to "" instead of "^std::".

This patch is just a quick fix that sets the regex back to the original value
to make the tests pass.

In total these 3 setting values have changed with D75537 and also need to be
fixed (even though they don't seem to break any tests).
  target.process.thread.step-avoid-regexp (regex) -> from '^std::' to empty 
string
  platform.module-cache-directory (file) -> from "~/.lldb/module_cache" to 
empty string
  script-lang (enum) -> from 'default' to 'python'

Added: 


Modified: 
lldb/include/lldb/Interpreter/OptionValueRegex.h

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/OptionValueRegex.h 
b/lldb/include/lldb/Interpreter/OptionValueRegex.h
index caeab69b5ef3..b09b8414d5bf 100644
--- a/lldb/include/lldb/Interpreter/OptionValueRegex.h
+++ b/lldb/include/lldb/Interpreter/OptionValueRegex.h
@@ -17,7 +17,8 @@ namespace lldb_private {
 class OptionValueRegex : public OptionValue {
 public:
   OptionValueRegex(const char *value = nullptr)
-  : OptionValue(), m_regex(llvm::StringRef::withNullAsEmpty(value)) {}
+  : OptionValue(), m_regex(llvm::StringRef::withNullAsEmpty(value)),
+m_default_regex_str(llvm::StringRef::withNullAsEmpty(value).str()) {}
 
   ~OptionValueRegex() override = default;
 
@@ -36,7 +37,7 @@ class OptionValueRegex : public OptionValue {
  VarSetOperationType = eVarSetOperationAssign) = delete;
 
   bool Clear() override {
-m_regex = RegularExpression();
+m_regex = RegularExpression(m_default_regex_str);
 m_value_was_set = false;
 return true;
   }
@@ -59,6 +60,7 @@ class OptionValueRegex : public OptionValue {
 
 protected:
   RegularExpression m_regex;
+  std::string m_default_regex_str;
 };
 
 } // namespace lldb_private



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

[Lldb-commits] [PATCH] D75537: Clear all settings during a test's setUp

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

Sure, I can take care of it. It seems `OptionValueRegex::Clear` is always 
resetting to an empty regex. I guess the same goes for the other option values 
that are failing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75537



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

[Lldb-commits] [PATCH] D75537: Clear all settings during a test's setUp

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added a comment.

Thank you for details, I'm looking at these failures, however, I'm not able to 
debug on the macOS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75537



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

[Lldb-commits] [PATCH] D75537: Clear all settings during a test's setUp

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I guess now all the shady hacks are coming out. The following settings change 
their value after calling settings clear after startup in the test:

  target.process.thread.step-avoid-regexp (regex) -> from '^std::' to empty 
string
  platform.module-cache-directory (file) -> from 
'"/Users/teemperor/.lldb/module_cache"' to empty string
  script-lang (enum) -> from 'default' to 'python'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75537



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

[Lldb-commits] [PATCH] D75537: Clear all settings during a test's setUp

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

Somehow this broke the macOS LLDB bot:

  Failing Tests (2):
  lldb-api :: functionalities/inline-stepping/TestInlineStepping.py
  lldb-api :: 
lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75537



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

[Lldb-commits] [lldb] e3fc6b3 - [lldb][NFC] Fix unsigned/signed comparison warning in SymbolFileDWARFTest.cpp

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-03-12T15:30:11+01:00
New Revision: e3fc6b3c346f583adbdb38b0935cc73439aaad99

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

LOG: [lldb][NFC] Fix unsigned/signed comparison warning in 
SymbolFileDWARFTest.cpp

offset_t is unsigned, so if the RHS is signed we get a warning from clang:
warning: comparison of integers of different signs: 'const unsigned long 
long' and 'const int'

Added: 


Modified: 
lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp

Removed: 




diff  --git a/lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp 
b/lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
index f1010a100bfd..8bf019ea9ed6 100644
--- a/lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
@@ -344,5 +344,5 @@ TEST_F(SymbolFileDWARFTests, 
ParseArangesNonzeroSegmentSize) {
   EXPECT_TRUE(bool(error));
   EXPECT_EQ("segmented arange entries are not supported",
 llvm::toString(std::move(error)));
-  EXPECT_EQ(off, 12); // Parser should read no further than the segment size
+  EXPECT_EQ(off, 12U); // Parser should read no further than the segment size
 }



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

[Lldb-commits] [PATCH] D75537: Clear all settings during a test's setUp

[Tatyana Krasnukha via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdf90a15b1ac9: [lldb] Clear all settings during a tests 
setUp (authored by tatyana-krasnukha).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75537

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/Options.td
  lldb/test/API/commands/settings/TestSettings.py

Index: lldb/test/API/commands/settings/TestSettings.py
===
--- lldb/test/API/commands/settings/TestSettings.py
+++ lldb/test/API/commands/settings/TestSettings.py
@@ -520,6 +520,32 @@
 self.expect("settings remove ''", error=True,
 substrs=["'settings remove' command requires a valid variable name"])
 
+def test_settings_clear_all(self):
+# Change a dictionary.
+self.runCmd("settings set target.env-vars a=1 b=2 c=3")
+# Change an array.
+self.runCmd("settings set target.run-args a1 b2 c3")
+# Change a single boolean value.
+self.runCmd("settings set auto-confirm true")
+# Change a single integer value.
+self.runCmd("settings set tab-size 2")
+
+# Clear everything.
+self.runCmd("settings clear --all")
+
+# Check that settings have their default values after clearing.
+self.expect("settings show target.env-vars", patterns=['^target.env-vars \(dictionary of strings\) =\s*$'])
+self.expect("settings show target.run-args", patterns=['^target.run-args \(arguments\) =\s*$'])
+self.expect("settings show auto-confirm", substrs=["false"])
+self.expect("settings show tab-size", substrs=["4"])
+
+# Check that the command fails if we combine '--all' option with any arguments.
+self.expect(
+"settings clear --all auto-confirm",
+COMMAND_FAILED_AS_EXPECTED,
+error=True,
+substrs=["'settings clear --all' doesn't take any arguments"])
+
 def test_all_settings_exist(self):
 self.expect("settings show",
 substrs=["auto-confirm",
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -39,6 +39,11 @@
 Desc<"The file from which to read the settings.">;
 }
 
+let Command = "settings clear" in {
+  def setclear_all : Option<"all", "a">,
+Desc<"Clear all settings.">;
+}
+
 let Command = "breakpoint list" in {
   // FIXME: We need to add an "internal" command, and then add this sort of
   // thing to it. But I need to see it for now, and don't want to wait.
Index: lldb/source/Commands/CommandObjectSettings.cpp
===
--- lldb/source/Commands/CommandObjectSettings.cpp
+++ lldb/source/Commands/CommandObjectSettings.cpp
@@ -1043,13 +1043,16 @@
 };
 
 // CommandObjectSettingsClear
+#define LLDB_OPTIONS_settings_clear
+#include "CommandOptions.inc"
 
 class CommandObjectSettingsClear : public CommandObjectParsed {
 public:
   CommandObjectSettingsClear(CommandInterpreter )
   : CommandObjectParsed(
 interpreter, "settings clear",
-"Clear a debugger setting array, dictionary, or string.", nullptr) {
+"Clear a debugger setting array, dictionary, or string. "
+"If '-a' option is specified, it clears all settings.", nullptr) {
 CommandArgumentEntry arg;
 CommandArgumentData var_name_arg;
 
@@ -1077,11 +1080,53 @@
   request, nullptr);
   }
 
+   Options *GetOptions() override { return _options; }
+
+  class CommandOptions : public Options {
+  public:
+CommandOptions() = default;
+
+~CommandOptions() override = default;
+
+Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
+  ExecutionContext *execution_context) override {
+  const int short_option = m_getopt_table[option_idx].val;
+  switch (short_option) {
+  case 'a':
+m_clear_all = true;
+break;
+  default:
+llvm_unreachable("Unimplemented option");
+  }
+  return Status();
+}
+
+void OptionParsingStarting(ExecutionContext *execution_context) override {
+  m_clear_all = false;
+}
+
+llvm::ArrayRef GetDefinitions() override {
+  return llvm::makeArrayRef(g_settings_clear_options);
+}
+
+bool m_clear_all = false;
+  };
+
 protected:
   bool DoExecute(Args , CommandReturnObject ) override {
 result.SetStatus(eReturnStatusSuccessFinishNoResult);
 const size_t argc = command.GetArgumentCount();
 
+if (m_options.m_clear_all) {
+  if (argc != 0) {
+result.AppendError("'settings clear --all' doesn't take any arguments");
+

[Lldb-commits] [lldb] df90a15 - [lldb] Clear all settings during a test's setUp

[Tatyana Krasnukha via lldb-commits]

Author: Tatyana Krasnukha
Date: 2020-03-12T16:30:26+03:00
New Revision: df90a15b1ac938559a8c3af12126559c1e1e9558

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

LOG: [lldb] Clear all settings during a test's setUp

Global properties are shared between debugger instances and
if a test doesn't clear changes in settings it made,
this leads to side effects in other tests.

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/source/Commands/CommandObjectSettings.cpp
lldb/source/Commands/Options.td
lldb/test/API/commands/settings/TestSettings.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index a9d6e50ce01f..cd48747c3557 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -687,6 +687,9 @@ def getSourcePath(self, name):
 @classmethod
 def setUpCommands(cls):
 commands = [
+# First of all, clear all settings to have clean state of global 
properties.
+"settings clear -all",
+
 # Disable Spotlight lookup. The testsuite creates
 # 
diff erent binaries with the same UUID, because they only
 # 
diff er in the debug info, which is not being hashed.

diff  --git a/lldb/source/Commands/CommandObjectSettings.cpp 
b/lldb/source/Commands/CommandObjectSettings.cpp
index 4d64ae428bad..cc1080c8cc0c 100644
--- a/lldb/source/Commands/CommandObjectSettings.cpp
+++ b/lldb/source/Commands/CommandObjectSettings.cpp
@@ -1043,13 +1043,16 @@ class CommandObjectSettingsAppend : public 
CommandObjectRaw {
 };
 
 // CommandObjectSettingsClear
+#define LLDB_OPTIONS_settings_clear
+#include "CommandOptions.inc"
 
 class CommandObjectSettingsClear : public CommandObjectParsed {
 public:
   CommandObjectSettingsClear(CommandInterpreter )
   : CommandObjectParsed(
 interpreter, "settings clear",
-"Clear a debugger setting array, dictionary, or string.", nullptr) 
{
+"Clear a debugger setting array, dictionary, or string. "
+"If '-a' option is specified, it clears all settings.", nullptr) {
 CommandArgumentEntry arg;
 CommandArgumentData var_name_arg;
 
@@ -1077,11 +1080,53 @@ class CommandObjectSettingsClear : public 
CommandObjectParsed {
   request, nullptr);
   }
 
+   Options *GetOptions() override { return _options; }
+
+  class CommandOptions : public Options {
+  public:
+CommandOptions() = default;
+
+~CommandOptions() override = default;
+
+Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
+  ExecutionContext *execution_context) override {
+  const int short_option = m_getopt_table[option_idx].val;
+  switch (short_option) {
+  case 'a':
+m_clear_all = true;
+break;
+  default:
+llvm_unreachable("Unimplemented option");
+  }
+  return Status();
+}
+
+void OptionParsingStarting(ExecutionContext *execution_context) override {
+  m_clear_all = false;
+}
+
+llvm::ArrayRef GetDefinitions() override {
+  return llvm::makeArrayRef(g_settings_clear_options);
+}
+
+bool m_clear_all = false;
+  };
+
 protected:
   bool DoExecute(Args , CommandReturnObject ) override {
 result.SetStatus(eReturnStatusSuccessFinishNoResult);
 const size_t argc = command.GetArgumentCount();
 
+if (m_options.m_clear_all) {
+  if (argc != 0) {
+result.AppendError("'settings clear --all' doesn't take any 
arguments");
+result.SetStatus(eReturnStatusFailed);
+return false;
+  }
+  GetDebugger().GetValueProperties()->Clear();
+  return result.Succeeded();
+}
+
 if (argc != 1) {
   result.AppendError("'settings clear' takes exactly one argument");
   result.SetStatus(eReturnStatusFailed);
@@ -1106,6 +1151,9 @@ class CommandObjectSettingsClear : public 
CommandObjectParsed {
 
 return result.Succeeded();
   }
+
+  private:
+CommandOptions m_options;
 };
 
 // CommandObjectMultiwordSettings

diff  --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 1456630c892f..fa8f5224cf53 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -39,6 +39,11 @@ let Command = "settings read" in {
 Desc<"The file from which to read the settings.">;
 }
 
+let Command = "settings clear" in {
+  def setclear_all : Option<"all", "a">,
+Desc<"Clear all settings.">;
+}
+
 let Command = "breakpoint list" in {
   // FIXME: We need to add an "internal" command, and then add this sort of
   // thing to it. But I need 

[Lldb-commits] [PATCH] D75925: [lldb] reject `.debug_arange` sections with nonzero segment size

[Luke Drummond via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0fa3320931e9: [lldb] reject `.debug_arange` sections with 
nonzero segment size (authored by ldrumm).

Changed prior to commit:
  https://reviews.llvm.org/D75925?vs=249663=249913#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75925

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
  lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp


Index: lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
@@ -18,6 +18,7 @@
 #include "Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDataExtractor.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h"
+#include "Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
 #include "Plugins/SymbolFile/PDB/SymbolFilePDB.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
@@ -310,3 +311,38 @@
   EXPECT_EQ("abbreviation declaration attribute list not terminated with a "
 "null entry", llvm::toString(std::move(error)));
 }
+
+TEST_F(SymbolFileDWARFTests, ParseArangesNonzeroSegmentSize) {
+  // This `.debug_aranges` table header is a valid 32bit big-endian section
+  // according to the DWARFv5 spec:6.2.1, but contains segment selectors which
+  // are not supported by lldb, and should be gracefully rejected
+  const unsigned char binary_data[] = {
+  0, 0, 0, 41, // unit_length (length field not including this field 
itself)
+  0, 2,// DWARF version number (half)
+  0, 0, 0, 0, // offset into the .debug_info_table (ignored for the 
purposes
+  // of this test
+  4,  // address size
+  1,  // segment size
+  // alignment for the first tuple which "begins at an offset that is a
+  // multiple of the size of a single tuple". Tuples are nine bytes in this
+  // example.
+  0, 0, 0, 0, 0, 0,
+  // BEGIN TUPLES
+  1, 0, 0, 0, 4, 0, 0, 0,
+  1, // a 1byte object starting at address 4 in segment 1
+  0, 0, 0, 0, 4, 0, 0, 0,
+  1, // a 1byte object starting at address 4 in segment 0
+  // END TUPLES
+  0, 0, 0, 0, 0, 0, 0, 0, 0 // terminator
+  };
+  DWARFDataExtractor data;
+  data.SetData(static_cast(binary_data), sizeof binary_data,
+   lldb::ByteOrder::eByteOrderBig);
+  DWARFDebugArangeSet debug_aranges;
+  offset_t off = 0;
+  llvm::Error error = debug_aranges.extract(data, );
+  EXPECT_TRUE(bool(error));
+  EXPECT_EQ("segmented arange entries are not supported",
+llvm::toString(std::move(error)));
+  EXPECT_EQ(off, 12); // Parser should read no further than the segment size
+}
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
@@ -63,7 +63,8 @@
   // 1 - the version looks good
   // 2 - the address byte size looks plausible
   // 3 - the length seems to make sense
-  // size looks plausible
+  // 4 - size looks plausible
+  // 5 - the arange tuples do not contain a segment field
   if (m_header.version < 2 || m_header.version > 5)
 return llvm::make_error(
 "Invalid arange header version");
@@ -81,6 +82,10 @@
 return llvm::make_error(
 "Invalid arange header length");
 
+  if (m_header.seg_size)
+return llvm::make_error(
+"segmented arange entries are not supported");
+
   // The first tuple following the header in each set begins at an offset
   // that is a multiple of the size of a single tuple (that is, twice the
   // size of an address). The header is padded, if necessary, to the


Index: lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
@@ -18,6 +18,7 @@
 #include "Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDataExtractor.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h"
+#include "Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
 #include "Plugins/SymbolFile/PDB/SymbolFilePDB.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
@@ -310,3 +311,38 @@
   EXPECT_EQ("abbreviation declaration attribute list not terminated with a "
 "null entry", llvm::toString(std::move(error)));
 }
+
+TEST_F(SymbolFileDWARFTests, ParseArangesNonzeroSegmentSize) {
+  // This 

[Lldb-commits] [PATCH] D75537: Clear all settings during a test's setUp

[Pavel Labath via Phabricator via lldb-commits]

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

Awesome, thanks.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D75537



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

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

[Pavel Labath via Phabricator via lldb-commits]

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

This looks good to me, though I can't say I'm that familiar with thread plan 
intricacies.




Comment at: lldb/include/lldb/Target/ThreadPlanStack.h:118
+
+  void Update(ThreadList _threads);
+

Delete the dangling declaration as the current patch does not implement this.



Comment at: lldb/include/lldb/Target/ThreadPlanStack.h:126-130
+auto result = m_plans_list.find(tid);
+if (result == m_plans_list.end())
+  return false;
+result->second.ThreadDestroyed(nullptr);
+m_plans_list.erase(result);

Any chance of calling `ThreadDestroyed` from ThreadPlanStack destructor, so 
this can just be `m_plan_list.erase(tid)` ?



Comment at: lldb/source/Target/ThreadPlanStack.cpp:373
+  }
+}

jingham wrote:
> labath wrote:
> > jingham wrote:
> > > labath wrote:
> > > > This is the usual place where one would add a llvm_unreachable. Some 
> > > > compilers do not consider the above switch to be fully covered (and if 
> > > > we are to be fully pedantic, it isn't) and will complain about falling 
> > > > off of the end of a non-void function.
> > > I added a default, but what do you mean by "if we are to be fully 
> > > pedantic, it isn't"?  It's an enum with 3 cases, all of which are listed. 
> > >  How is that not fully covered?
> > The first sentence of [[ https://en.cppreference.com/w/cpp/language/enum | 
> > cppreference enum definition ]] could be helpful here:
> > ```
> > An enumeration is a distinct type whose value is restricted to a range of 
> > values (see below for details), which may include several explicitly named 
> > constants ("enumerators").
> > ```
> > What exactly is the "range of values" of an enum is a complex topic, but 
> > suffice to say it usually includes more than the named constants one has 
> > mentioned in the enum definition (imagine bitfield enums).
> > Clang takes the "common sense" approach and assumes that if one does a 
> > switch over an enum, he expects it to always have one of the named 
> > constants. Gcc follows a stricter interpretation and concludes that there 
> > are execution flows which do not return a value from this function and 
> > issues a warning. Which behavior is more correct is debatable...
> > 
> > Unfortunately adding a default case is not the right solution here (though 
> > I can see how that could be understood from my comment -- sorry), because 
> > clang will then warn you about putting a default statement in what it 
> > considers to be a fully covered switch. The right solution is to put the 
> > llvm_unreachable call *after* the switch statement, which is the layout 
> > that will make everyone happy. You don't need (or shouldn't) put the return 
> > statement because the compilers know that the macro terminates the progream.
> > 
> > I'm sorry that this turned into such a lesson, but since you were using the 
> > macro in an atypical way, I wanted to illustrate what is the more common 
> > use of it.
> The "may" in that sentence is apparently the crucial bit, but it didn't 
> explain why an enum specified only by named constants might have other 
> possible values.  I read a little further on, but didn't get to the part 
> where it explains that.  Seems to me in this case the compiler shouldn't have 
> to to guess about the extent of the valid values of this enum and then be 
> conservative or not in handling the results of its guess.  I don't see the 
> point of an enum if a legally constructed enum value (not cast from some 
> random value) isn't exhausted by the constants used to define it.  That just 
> seems weird.
> 
> But arguing with C++ compiler diagnostics isn't a productive use of time, and 
> I feel like too deep an understanding of the C++ type system's mysteries is 
> not good for one's mental stability.  So I'm more than content to cargo cult 
> this one...
The rough rule is that for enums without a fixed underlying type, the range of 
valid values is those that are representable by the smallest bitfield capable 
of holding all enumerators of that enum. For a fixed underlying type, the range 
is the whole type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75880



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

[Lldb-commits] [lldb] 0fa3320 - [lldb] reject `.debug_arange` sections with nonzero segment size

[Luke Drummond via lldb-commits]

Author: Luke Drummond
Date: 2020-03-12T12:22:50Z
New Revision: 0fa3320931e93d658f68c1f19eb45b922158b61e

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

LOG: [lldb] reject `.debug_arange` sections with nonzero segment size

If a producer emits a nonzero segment size, `lldb` will silently read
incorrect values and crash, or do something worse later as the tuple
size is expected to be 2, rather than 3.

Neither LLVM, nor GCC produce segmented aranges, but this dangerous case
should still be checked and handled.

Reviewed by: clayborg, labath
Differential Revision: https://reviews.llvm.org/D75925
Subscribers: lldb-commits
Tags: #lldb

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
index b3f056d75593..728cefe620a5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
@@ -63,7 +63,8 @@ llvm::Error DWARFDebugArangeSet::extract(const 
DWARFDataExtractor ,
   // 1 - the version looks good
   // 2 - the address byte size looks plausible
   // 3 - the length seems to make sense
-  // size looks plausible
+  // 4 - size looks plausible
+  // 5 - the arange tuples do not contain a segment field
   if (m_header.version < 2 || m_header.version > 5)
 return llvm::make_error(
 "Invalid arange header version");
@@ -81,6 +82,10 @@ llvm::Error DWARFDebugArangeSet::extract(const 
DWARFDataExtractor ,
 return llvm::make_error(
 "Invalid arange header length");
 
+  if (m_header.seg_size)
+return llvm::make_error(
+"segmented arange entries are not supported");
+
   // The first tuple following the header in each set begins at an offset
   // that is a multiple of the size of a single tuple (that is, twice the
   // size of an address). The header is padded, if necessary, to the

diff  --git a/lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp 
b/lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
index c971eda2d46e..f1010a100bfd 100644
--- a/lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
@@ -18,6 +18,7 @@
 #include "Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDataExtractor.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h"
+#include "Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
 #include "Plugins/SymbolFile/PDB/SymbolFilePDB.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
@@ -310,3 +311,38 @@ TEST_F(SymbolFileDWARFTests, TestAbbrevMissingTerminator) {
   EXPECT_EQ("abbreviation declaration attribute list not terminated with a "
 "null entry", llvm::toString(std::move(error)));
 }
+
+TEST_F(SymbolFileDWARFTests, ParseArangesNonzeroSegmentSize) {
+  // This `.debug_aranges` table header is a valid 32bit big-endian section
+  // according to the DWARFv5 spec:6.2.1, but contains segment selectors which
+  // are not supported by lldb, and should be gracefully rejected
+  const unsigned char binary_data[] = {
+  0, 0, 0, 41, // unit_length (length field not including this field 
itself)
+  0, 2,// DWARF version number (half)
+  0, 0, 0, 0, // offset into the .debug_info_table (ignored for the 
purposes
+  // of this test
+  4,  // address size
+  1,  // segment size
+  // alignment for the first tuple which "begins at an offset that is a
+  // multiple of the size of a single tuple". Tuples are nine bytes in this
+  // example.
+  0, 0, 0, 0, 0, 0,
+  // BEGIN TUPLES
+  1, 0, 0, 0, 4, 0, 0, 0,
+  1, // a 1byte object starting at address 4 in segment 1
+  0, 0, 0, 0, 4, 0, 0, 0,
+  1, // a 1byte object starting at address 4 in segment 0
+  // END TUPLES
+  0, 0, 0, 0, 0, 0, 0, 0, 0 // terminator
+  };
+  DWARFDataExtractor data;
+  data.SetData(static_cast(binary_data), sizeof binary_data,
+   lldb::ByteOrder::eByteOrderBig);
+  DWARFDebugArangeSet debug_aranges;
+  offset_t off = 0;
+  llvm::Error error = debug_aranges.extract(data, );
+  EXPECT_TRUE(bool(error));
+  EXPECT_EQ("segmented arange entries are not supported",
+llvm::toString(std::move(error)));
+  EXPECT_EQ(off, 12); // Parser should read no further than the segment size
+}



___
lldb-commits mailing list
lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D75537: Clear all settings during a test's setUp

[Tatyana Krasnukha via Phabricator via lldb-commits]

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

Addressed comments


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D75537

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/Options.td
  lldb/test/API/commands/settings/TestSettings.py

Index: lldb/test/API/commands/settings/TestSettings.py
===
--- lldb/test/API/commands/settings/TestSettings.py
+++ lldb/test/API/commands/settings/TestSettings.py
@@ -520,6 +520,32 @@
 self.expect("settings remove ''", error=True,
 substrs=["'settings remove' command requires a valid variable name"])
 
+def test_settings_clear_all(self):
+# Change a dictionary.
+self.runCmd("settings set target.env-vars a=1 b=2 c=3")
+# Change an array.
+self.runCmd("settings set target.run-args a1 b2 c3")
+# Change a single boolean value.
+self.runCmd("settings set auto-confirm true")
+# Change a single integer value.
+self.runCmd("settings set tab-size 2")
+
+# Clear everything.
+self.runCmd("settings clear --all")
+
+# Check that settings have their default values after clearing.
+self.expect("settings show target.env-vars", patterns=['^target.env-vars \(dictionary of strings\) =\s*$'])
+self.expect("settings show target.run-args", patterns=['^target.run-args \(arguments\) =\s*$'])
+self.expect("settings show auto-confirm", substrs=["false"])
+self.expect("settings show tab-size", substrs=["4"])
+
+# Check that the command fails if we combine '--all' option with any arguments.
+self.expect(
+"settings clear --all auto-confirm",
+COMMAND_FAILED_AS_EXPECTED,
+error=True,
+substrs=["'settings clear --all' doesn't take any arguments"])
+
 def test_all_settings_exist(self):
 self.expect("settings show",
 substrs=["auto-confirm",
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -39,6 +39,11 @@
 Desc<"The file from which to read the settings.">;
 }
 
+let Command = "settings clear" in {
+  def setclear_all : Option<"all", "a">,
+Desc<"Clear all settings.">;
+}
+
 let Command = "breakpoint list" in {
   // FIXME: We need to add an "internal" command, and then add this sort of
   // thing to it. But I need to see it for now, and don't want to wait.
Index: lldb/source/Commands/CommandObjectSettings.cpp
===
--- lldb/source/Commands/CommandObjectSettings.cpp
+++ lldb/source/Commands/CommandObjectSettings.cpp
@@ -1043,13 +1043,16 @@
 };
 
 // CommandObjectSettingsClear
+#define LLDB_OPTIONS_settings_clear
+#include "CommandOptions.inc"
 
 class CommandObjectSettingsClear : public CommandObjectParsed {
 public:
   CommandObjectSettingsClear(CommandInterpreter )
   : CommandObjectParsed(
 interpreter, "settings clear",
-"Clear a debugger setting array, dictionary, or string.", nullptr) {
+"Clear a debugger setting array, dictionary, or string. "
+"If '-a' option is specified, it clears all settings.", nullptr) {
 CommandArgumentEntry arg;
 CommandArgumentData var_name_arg;
 
@@ -1077,11 +1080,53 @@
   request, nullptr);
   }
 
+   Options *GetOptions() override { return _options; }
+
+  class CommandOptions : public Options {
+  public:
+CommandOptions() = default;
+
+~CommandOptions() override = default;
+
+Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
+  ExecutionContext *execution_context) override {
+  const int short_option = m_getopt_table[option_idx].val;
+  switch (short_option) {
+  case 'a':
+m_clear_all = true;
+break;
+  default:
+llvm_unreachable("Unimplemented option");
+  }
+  return Status();
+}
+
+void OptionParsingStarting(ExecutionContext *execution_context) override {
+  m_clear_all = false;
+}
+
+llvm::ArrayRef GetDefinitions() override {
+  return llvm::makeArrayRef(g_settings_clear_options);
+}
+
+bool m_clear_all = false;
+  };
+
 protected:
   bool DoExecute(Args , CommandReturnObject ) override {
 result.SetStatus(eReturnStatusSuccessFinishNoResult);
 const size_t argc = command.GetArgumentCount();
 
+if (m_options.m_clear_all) {
+  if (argc != 0) {
+result.AppendError("'settings clear --all' doesn't take any arguments");
+result.SetStatus(eReturnStatusFailed);
+return false;
+  }
+  GetDebugger().GetValueProperties()->Clear();

[Lldb-commits] [PATCH] D76045: [lldb/API] Make Launch(Simple) use args and env from target properties

[Pavel Labath via Phabricator via lldb-commits]

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

sounds reasonable.




Comment at: lldb/include/lldb/API/SBTarget.h:130-132
+  /// The environment array. If this isn't provided, the default
+  /// environment values (provided through `settings set
+  /// target.env-vars`) will be used.

Make it explicit that "isn't provided" means nullptr. And maybe mention that 
one can get an empty environment by passing an empty array. And do the same for 
the argv array. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76045



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

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D74636#1918110 , @clayborg wrote:

> In D74636#1916356 , @labath wrote:
>
> > There's a `target.inherit-env` setting in lldb (which I believe also works 
> > correctly for remote launches). Could you use that instead of 
> > reimplementing the feature in vscode?
>
>
> The real question is if we want launching to fail when the user tries to 
> enable this for remote targets? If we don't care if it fails, then we can use 
> "target.inherit-env", but I kind of like that it does fail as it clearly 
> conveys to the user that their environment won't be passed along instead of 
> just ignoring the request. Thoughts?


The real question for me is what do we expect this setting to do. If the 
intention is to pass the host environment, then that doesn't generally make 
sense for remote targets, and failing would be good. But if we just want to 
ensure it inherits "a suitable" environment, then there's no reason to fail. I 
believe this is what `target.inherit-env` will do for remote launches, where it 
will take the environment from the remote platform, and "inherit" that. I think 
it would make sense if this setting and `target.inherit-env` behaved the same 
way...

In D74636#1918682 , @wallace wrote:

> Regarding implementation:
>
> The target.inherit-env setting is only effectively used by 
> CommandObjectProcess when launching the target from the command line. 
> lldb-vscode is not following the same codepath and not using that property.
>
> What about exposing the Platform's environment in the API and merging 
> SBPlatform->GetEnvironment() with the launch.json's environment if 
> inheritEnvironment is True? That would be very similar to what is doing 
> CommandObjectProcess


SBPlatform::GetEnvironment sounds perfectly reasonable to me.

I am also wondering if D76009  is relevant in 
any way here...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74636



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

[Lldb-commits] [PATCH] D76009: [lldb/Target] Initialize new targets environment variables from target.env-vars

[Pavel Labath via Phabricator via lldb-commits]

labath removed a reviewer: labath.
labath added a comment.
This revision now requires review to proceed.

Actually, hang on.

> One existing test had to be modified, because the initialization of
>  the environment properties now take place at the time the target is
>  created, not at the first use of the environment (usually launch
>  time).

Does this mean that the target-local value of the `target.inherit-env` setting 
no longer has any effect? Currently I can set it after creating a target (but 
before launching) to stop the process inheriting the default environment:

  (lldb) file /usr/bin/env
  Current executable set to '/usr/bin/env' (x86_64).
  (lldb) set set target.inherit-env false
  (lldb) pr la
  Process 26684 launched: '/usr/bin/env' (x86_64)
  Process 26684 exited with status = 0 (0x) 

I take it that after this, that will no longer be possible? It would still be 
possible to do that by setting the global value of the setting before creating 
a target, but the target-local setting would be useless (?)

I'm not really sure how these settings are supposed to work, but this does not 
seem ideal to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76009



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

[Lldb-commits] [PATCH] D76009: [lldb/Target] Initialize new targets environment variables from target.env-vars

[Pavel Labath via Phabricator via lldb-commits]

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

Another option might be to make TargetProperties a member instead of a base 
class. I don't think the current setup makes for a particularly clean design. 
If `TargetProperties` is among the last Target members being initialized, then 
calling the callbacks from the constructor should "just work".




Comment at: lldb/include/lldb/Target/Target.h:214
 
+  void RunCallbacks();
+

This name is very nondescript. I think it would be better to name this 
according to what it does, instead of how it does it. 
`LoadDefaultPropertyValues()` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76009



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

[Lldb-commits] [PATCH] D75925: [lldb] reject `.debug_arange` sections with nonzero segment size

[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, just run clang-format before committing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75925



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

[Lldb-commits] [PATCH] D76004: [lldb] Add YAML traits for ArchSpec and ProcessInstanceInfo

[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. The mapping vs sequence traits thingy is not that important, but 
since we do have the "dump" command, it may be worthwhile to spent a bit of 
time implementing a slightly more complicated traits class so that the yaml 
output looks nicer. (Another reason against wrapping standard containters in 
custom XXXList classes)




Comment at: lldb/source/Utility/ProcessInfo.cpp:348-351
+void llvm::yaml::MappingTraits::mapping(
+IO , ProcessInstanceInfoList ) {
+  io.mapRequired("processes", List.m_infos);
+}

Maybe define this as a `SequenceTraits`? I believe you 
could implement the required functions by simply forwarding them to 
`SequenceTraits>::whatever`


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D76004



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

[Lldb-commits] [PATCH] D76002: [lldb] Add YAML traits for ConstString and FileSpec

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Utility/FileSpec.h:401
 protected:
+  template  friend struct llvm::yaml::MappingTraits;
+

BTW, you can also friend a specific template instantiation: `friend struct 
llvm::yaml::MappingTraits`.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D76002



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

[Lldb-commits] [PATCH] D76002: [lldb] Add YAML traits for ConstString and FileSpec

[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 fine, we just need to avoid ODR violations




Comment at: lldb/include/lldb/Utility/FileSpec.h:444
+namespace yaml {
+template <> struct ScalarEnumerationTraits {
+  static void enumeration(IO , lldb_private::FileSpec::Style );

FileSpec::Style is a typedef for `llvm::sys::path::Style`. Typedefs are not 
separate types so this code is defining a specialization for a type it does not 
own.

I think the simplest solution for that is to define a 
`LLVM_YAML_STRONG_TYPEDEF(FileSpec::Style, MyStyle)`
and then do something like
```
MyStyle style = f.m_style;
io.mapRequired("style", style);
f.m_style = style
```
in `MappingTraits`


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D76002



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

[Lldb-commits] [PATCH] D75979: [lldb] Remove unimplemented StackFrame::BehavesLikeZerothFrame

[Anton Kolesov via Phabricator via lldb-commits]

anton.kolesov updated this revision to Diff 249844.
anton.kolesov retitled this revision from "[lldb] Implement 
StackFrame::BehavesLikeZerothFrame" to "[lldb] Remove unimplemented 
StackFrame::BehavesLikeZerothFrame".
anton.kolesov edited the summary of this revision.
anton.kolesov added a comment.

Removed function declaration as requested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75979

Files:
  lldb/include/lldb/Target/StackFrame.h


Index: lldb/include/lldb/Target/StackFrame.h
===
--- lldb/include/lldb/Target/StackFrame.h
+++ lldb/include/lldb/Target/StackFrame.h
@@ -367,12 +367,6 @@
   /// may have limited support for inspecting variables.
   bool IsArtificial() const;
 
-  /// Query whether this frame behaves like the zeroth frame, in the sense
-  /// that its pc value might not immediately follow a call (and thus might
-  /// be the first address of its function).  True for actual frame zero as
-  /// well as any other frame with the same trait.
-  bool BehavesLikeZerothFrame() const;
-
   /// Query this frame to find what frame it is in this Thread's
   /// StackFrameList.
   ///
@@ -517,6 +511,11 @@
   bool m_cfa_is_valid; // Does this frame have a CFA?  Different from CFA ==
// LLDB_INVALID_ADDRESS
   Kind m_stack_frame_kind;
+
+  // Whether this frame behaves like the zeroth frame, in the sense
+  // that its pc value might not immediately follow a call (and thus might
+  // be the first address of its function). True for actual frame zero as
+  // well as any other frame with the same trait.
   bool m_behaves_like_zeroth_frame;
   lldb::VariableListSP m_variable_list_sp;
   ValueObjectList m_variable_list_value_objects; // Value objects for each


Index: lldb/include/lldb/Target/StackFrame.h
===
--- lldb/include/lldb/Target/StackFrame.h
+++ lldb/include/lldb/Target/StackFrame.h
@@ -367,12 +367,6 @@
   /// may have limited support for inspecting variables.
   bool IsArtificial() const;
 
-  /// Query whether this frame behaves like the zeroth frame, in the sense
-  /// that its pc value might not immediately follow a call (and thus might
-  /// be the first address of its function).  True for actual frame zero as
-  /// well as any other frame with the same trait.
-  bool BehavesLikeZerothFrame() const;
-
   /// Query this frame to find what frame it is in this Thread's
   /// StackFrameList.
   ///
@@ -517,6 +511,11 @@
   bool m_cfa_is_valid; // Does this frame have a CFA?  Different from CFA ==
// LLDB_INVALID_ADDRESS
   Kind m_stack_frame_kind;
+
+  // Whether this frame behaves like the zeroth frame, in the sense
+  // that its pc value might not immediately follow a call (and thus might
+  // be the first address of its function). True for actual frame zero as
+  // well as any other frame with the same trait.
   bool m_behaves_like_zeroth_frame;
   lldb::VariableListSP m_variable_list_sp;
   ValueObjectList m_variable_list_value_objects; // Value objects for each
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits