[Lldb-commits] [PATCH] D58566: [Reproducers] Add more logging capabilities to reproducer instrumentation

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 188492.
JDevlieghere added a comment.

Simplify record function


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

https://reviews.llvm.org/D58566

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

Index: lldb/source/Utility/ReproducerInstrumentation.cpp
===
--- lldb/source/Utility/ReproducerInstrumentation.cpp
+++ lldb/source/Utility/ReproducerInstrumentation.cpp
@@ -43,23 +43,33 @@
 }
 
 bool Registry::Replay(llvm::StringRef buffer) {
+#ifndef LLDB_REPRO_INSTR_TRACE
   Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_API);
+#endif
 
   Deserializer deserializer(buffer);
   while (deserializer.HasData(1)) {
 unsigned id = deserializer.Deserialize();
-LLDB_LOG(log, "Replaying function #{0}", id);
-m_ids[id]->operator()(deserializer);
+
+#ifndef LLDB_REPRO_INSTR_TRACE
+LLDB_LOG(log, "Replaying {0}: {1}", id, GetSignature(id));
+#else
+llvm::errs() << "Replaying " << id << ": " << GetSignature(id) << "\n";
+#endif
+
+GetReplayer(id)->operator()(deserializer);
   }
 
   return true;
 }
 
-void Registry::DoRegister(uintptr_t RunID, std::unique_ptr replayer) {
+void Registry::DoRegister(uintptr_t RunID, std::unique_ptr replayer,
+  SignatureStr signature) {
   const unsigned id = m_replayers.size() + 1;
   assert(m_replayers.find(RunID) == m_replayers.end());
   m_replayers[RunID] = std::make_pair(std::move(replayer), id);
-  m_ids[id] = m_replayers[RunID].first.get();
+  m_ids[id] =
+  std::make_pair(m_replayers[RunID].first.get(), std::move(signature));
 }
 
 unsigned Registry::GetID(uintptr_t addr) {
@@ -68,6 +78,21 @@
   return id;
 }
 
+std::string Registry::GetSignature(unsigned id) {
+  assert(m_ids.count(id) != 0 && "ID not in registry");
+  return m_ids[id].second.ToString();
+}
+
+Replayer *Registry::GetReplayer(unsigned id) {
+  assert(m_ids.count(id) != 0 && "ID not in registry");
+  return m_ids[id].first;
+}
+
+std::string Registry::SignatureStr::ToString() const {
+  return (result + (result.empty() ? "" : " ") + scope + "::" + name + args)
+  .str();
+}
+
 unsigned ObjectToIndex::GetIndexForObjectImpl(const void *object) {
   unsigned index = m_mapping.size() + 1;
   auto it = m_mapping.find(object);
Index: lldb/include/lldb/Utility/ReproducerInstrumentation.h
===
--- lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -18,15 +18,24 @@
 
 #include 
 
+// Define LLDB_REPRO_INSTR_TRACE to trace to stderr instead of LLDB's log
+// infrastructure. This is useful when you need to see traces before the logger
+// is initialized or enabled.
+#define LLDB_REPRO_INSTR_TRACE
+
 #define LLDB_REGISTER_CONSTRUCTOR(Class, Signature)\
-  Register(::doit)
+  Register(::doit, "", #Class,   \
+  #Class, #Signature)
 #define LLDB_REGISTER_METHOD(Result, Class, Method, Signature) \
-  Register(::method<::Method>::doit)
+  Register(::method<::Method>::doit,  \
+   #Result, #Class, #Method, #Signature)
 #define LLDB_REGISTER_METHOD_CONST(Result, Class, Method, Signature)   \
   Register(::method_const<::Method>::doit)
+   Signature const>::method_const<::Method>::doit,   \
+   #Result, #Class, #Method, #Signature)
 #define LLDB_REGISTER_STATIC_METHOD(Result, Class, Method, Signature)  \
-  Register(static_cast(::Method))
+  Register(static_cast(::Method), \
+ #Result, #Class, #Method, #Signature)
 
 #define LLDB_RECORD_CONSTRUCTOR(Class, Signature, ...) \
   LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "{0}",   \
@@ -224,6 +233,9 @@
 
   /// Deserialize and interpret value as T.
   template  T Deserialize() {
+#ifdef LLDB_REPRO_INSTR_TRACE
+llvm::errs() << "Deserializing with " << LLVM_PRETTY_FUNCTION << "\n";
+#endif
 return Read(typename serializer_tag::type());
   }
 
@@ -371,19 +383,41 @@
 /// IDs can be serialized and deserialized to replay a function. Functions need
 /// to be registered with the registry for this to work.
 class Registry {
+private:
+  struct SignatureStr {
+SignatureStr(llvm::StringRef result = {}, llvm::StringRef scope = {},
+ llvm::StringRef name = {}, llvm::StringRef args = {})
+: result(result), scope(scope), name(name), args(args) {}
+
+std::string ToString() const;
+
+llvm::StringRef result;
+llvm::StringRef scope;
+llvm::StringRef name;
+llvm::StringRef args;
+  };
+
 public:
   Registry() = default;
   virtual ~Registry() = default;
 
   /// Register a default replayer for a function.
-  template  void Register(Signature *f) {
-DoRegister(uintptr_t(f), 

[Lldb-commits] [PATCH] D58566: [Reproducers] Add more logging capabilities to reproducer instrumentation

[Jonas Devlieghere via Phabricator via lldb-commits]

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



Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:591
 
-LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "Recording ({0}) 
'{1}'",
+#ifndef LLDB_REPRO_INSTR_TRACE
+LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "Recording {0}: {1}",

labath wrote:
> aprantl wrote:
> > Who defines this macro? I'm asking because it's unusual for LLDB to emit 
> > anything on stderr.
> I'm guessing it's the kind of thing you define by editing the source/cmake 
> config, when you're really desperate to figure out what's going wrong.
Correct, see the #define on line 24. I'll comment it out before landing of 
course.


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

https://reviews.llvm.org/D58566



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

[Lldb-commits] [PATCH] D58664: [Utility] Fix ArchSpec.MergeFrom to correctly merge environments

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB354938: [Utility] Fix ArchSpec.MergeFrom to correctly 
merge environments (authored by xiaobai, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58664?vs=188474=188479#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58664

Files:
  source/Utility/ArchSpec.cpp
  unittests/Utility/ArchSpecTest.cpp


Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -134,22 +134,46 @@
 }
 
 TEST(ArchSpecTest, MergeFrom) {
-  ArchSpec A;
-  ArchSpec B("x86_64-pc-linux");
-
-  EXPECT_FALSE(A.IsValid());
-  ASSERT_TRUE(B.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
+  {
+ArchSpec A;
+ArchSpec B("x86_64-pc-linux");
 
-  A.MergeFrom(B);
-  ASSERT_TRUE(A.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+EXPECT_FALSE(A.IsValid());
+ASSERT_TRUE(B.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
+
+A.MergeFrom(B);
+ASSERT_TRUE(A.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+  }
+  {
+ArchSpec A("aarch64");
+ArchSpec B("aarch64--linux-android");
+
+EXPECT_TRUE(A.IsValid());
+EXPECT_TRUE(B.IsValid());
+
+EXPECT_EQ(llvm::Triple::ArchType::aarch64, 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::Android,
+  B.GetTriple().getEnvironment());
+
+A.MergeFrom(B);
+EXPECT_EQ(llvm::Triple::ArchType::aarch64, A.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(llvm::Triple::EnvironmentType::Android,
+  A.GetTriple().getEnvironment());
+  }
 }
 
 TEST(ArchSpecTest, MergeFromMachOUnknown) {
Index: source/Utility/ArchSpec.cpp
===
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -903,9 +903,8 @@
   UpdateCore();
   }
   if (!TripleEnvironmentWasSpecified() &&
-  other.TripleEnvironmentWasSpecified() && !TripleVendorWasSpecified()) {
-if (other.TripleVendorWasSpecified())
-  GetTriple().setEnvironment(other.GetTriple().getEnvironment());
+  other.TripleEnvironmentWasSpecified()) {
+GetTriple().setEnvironment(other.GetTriple().getEnvironment());
   }
   // If this and other are both arm ArchSpecs and this ArchSpec is a generic
   // "some kind of arm" spec but the other ArchSpec is a specific arm core,


Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -134,22 +134,46 @@
 }
 
 TEST(ArchSpecTest, MergeFrom) {
-  ArchSpec A;
-  ArchSpec B("x86_64-pc-linux");
-
-  EXPECT_FALSE(A.IsValid());
-  ASSERT_TRUE(B.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
+  {
+ArchSpec A;
+ArchSpec B("x86_64-pc-linux");
 
-  A.MergeFrom(B);
-  ASSERT_TRUE(A.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+EXPECT_FALSE(A.IsValid());
+ASSERT_TRUE(B.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, 

[Lldb-commits] [lldb] r354938 - [Utility] Fix ArchSpec.MergeFrom to correctly merge environments

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Tue Feb 26 16:47:39 2019
New Revision: 354938

URL: http://llvm.org/viewvc/llvm-project?rev=354938=rev
Log:
[Utility] Fix ArchSpec.MergeFrom to correctly merge environments

Summary:
This behavior was originally added in rL252264 (git commit 76a7f365da)
in order to be extra careful with handling platforms like watchos and tvos.
However, as far as triples go, those two (and others) are treated as OSes and
not environments, so that should not really apply here.

Additionally, this behavior is incorrect and can lead to incorrect ArchSpecs.
Because android is specified as an environment and not an OS, not propogating
the environment can lead to modules and targets being misidentified.

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

Modified:
lldb/trunk/source/Utility/ArchSpec.cpp
lldb/trunk/unittests/Utility/ArchSpecTest.cpp

Modified: lldb/trunk/source/Utility/ArchSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ArchSpec.cpp?rev=354938=354937=354938=diff
==
--- lldb/trunk/source/Utility/ArchSpec.cpp (original)
+++ lldb/trunk/source/Utility/ArchSpec.cpp Tue Feb 26 16:47:39 2019
@@ -903,9 +903,8 @@ void ArchSpec::MergeFrom(const ArchSpec
   UpdateCore();
   }
   if (!TripleEnvironmentWasSpecified() &&
-  other.TripleEnvironmentWasSpecified() && !TripleVendorWasSpecified()) {
-if (other.TripleVendorWasSpecified())
-  GetTriple().setEnvironment(other.GetTriple().getEnvironment());
+  other.TripleEnvironmentWasSpecified()) {
+GetTriple().setEnvironment(other.GetTriple().getEnvironment());
   }
   // If this and other are both arm ArchSpecs and this ArchSpec is a generic
   // "some kind of arm" spec but the other ArchSpec is a specific arm core,

Modified: lldb/trunk/unittests/Utility/ArchSpecTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/ArchSpecTest.cpp?rev=354938=354937=354938=diff
==
--- lldb/trunk/unittests/Utility/ArchSpecTest.cpp (original)
+++ lldb/trunk/unittests/Utility/ArchSpecTest.cpp Tue Feb 26 16:47:39 2019
@@ -134,22 +134,46 @@ TEST(ArchSpecTest, TestSetTriple) {
 }
 
 TEST(ArchSpecTest, MergeFrom) {
-  ArchSpec A;
-  ArchSpec B("x86_64-pc-linux");
+  {
+ArchSpec A;
+ArchSpec B("x86_64-pc-linux");
 
-  EXPECT_FALSE(A.IsValid());
-  ASSERT_TRUE(B.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
-
-  A.MergeFrom(B);
-  ASSERT_TRUE(A.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+EXPECT_FALSE(A.IsValid());
+ASSERT_TRUE(B.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
+
+A.MergeFrom(B);
+ASSERT_TRUE(A.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+  }
+  {
+ArchSpec A("aarch64");
+ArchSpec B("aarch64--linux-android");
+
+EXPECT_TRUE(A.IsValid());
+EXPECT_TRUE(B.IsValid());
+
+EXPECT_EQ(llvm::Triple::ArchType::aarch64, 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::Android,
+  B.GetTriple().getEnvironment());
+
+A.MergeFrom(B);
+EXPECT_EQ(llvm::Triple::ArchType::aarch64, A.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(llvm::Triple::EnvironmentType::Android,
+  A.GetTriple().getEnvironment());
+  }
 }
 
 TEST(ArchSpecTest, MergeFromMachOUnknown) {


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

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D58654#1410852 , @zturner wrote:

> In D58654#1410817 , @JDevlieghere 
> wrote:
>
> > I'm not a fan of introducing another library for this. Without a clear 
> > timeline of "fixing" Host it's basically as temporary as anything else in 
> > software development. I also think it's confusing (e,g, new files should go 
> > where?). If you really want a clean slate we should move everything in 
> > System that doesn't depend on anything other than Utility until Host is 
> > empty, but I'm sure everyone agrees that unrealistic and not worth the 
> > churn.
> >
> > For the record I also very much support a Host library instead of having 
> > everything in Utility. That being said, I could see the config file being 
> > special enough (in that everything depends on it) to be the exception.
> >
> > It seems like we could drop the dependencies on Core and Symbol by moving 
> > Symbols somewhere else. How much work would there be left after Pavel's 
> > patch?
>
>
> Moving Symbols somewhere else isn't exactly easy though.  There's a lot of 
> interdependencies that are slightly different on every Host platform, so it 
> creates a lot of breakage doing things this way.
>
> FWIW, I actually wouldn't mind moving everything out of Host into System 
> until Host is basically empty.  My next step after this patch was going to be 
> to move MainLoop followed by Socket into System (those are my actual goals 
> anyway).  In some ways, approaching the dependency problem from both 
> directions this way is more effective than approaching it from only one side, 
> as this way eventually Host becomes so small that you can just take whatever 
> is left over and have it be subsumed by some other target (probably Core or 
> Symbols).
>
> All of that said, I'm not as sold that having a separate library just for 
> Host is even terribly useful.  I'll go along with it if that's what everyone 
> wants, but at the end of the day, this library (whether it be called Host, or 
> System, or something else) will almost certainly depend on Utility, and 
> Utility will almost certainly depend on it (for the same reason that Utility 
> currently depends on llvm/Support).  This is why in LLVM/Support there are 
> both non-Host specific things, such as `StringExtras.h`, and host specific 
> things, such as `FileSystem.h`, `Thread.h`, etc.  I don't think that's 
> actually so bad in practice.
>
> So my vote is honestly still to just put this in Utility for consistency with 
> LLVM, but I can agree with `System` or something as part of a transition if 
> it gets us closer to being able to use these utility classes without a 
> dependency on the whole debugger.




In D58654#1411084 , @labath wrote:

> BTW, what's the reason that you have to have this split now? I understand its 
> attractiveness from a Zen perspective, but it seems to me that at the end of 
> the day, it doesn't have that much impact on the new code you're about to 
> write. It seems to me you could still enforce a clean layering on the new 
> code by just saying "only ever include stuff from Host or Utility". The fact 
> that Host transitively pulls in the kitchen sink is unfortunate, but I don't 
> think it should impact you much. And when we finally clean up Host (which is 
> something that we'll do eventually, and I hope not too far from now), then 
> the new code will magically stop depending on the whole world without any 
> extra effort.
>
> In D58654#1410852 , @zturner wrote:
>
> > In D58654#1410817 , @JDevlieghere 
> > wrote:
> >
> > > I'm not a fan of introducing another library for this. Without a clear 
> > > timeline of "fixing" Host it's basically as temporary as anything else in 
> > > software development. I also think it's confusing (e,g, new files should 
> > > go where?). If you really want a clean slate we should move everything in 
> > > System that doesn't depend on anything other than Utility until Host is 
> > > empty, but I'm sure everyone agrees that unrealistic and not worth the 
> > > churn.
> > >
> > > For the record I also very much support a Host library instead of having 
> > > everything in Utility. That being said, I could see the config file being 
> > > special enough (in that everything depends on it) to be the exception.
> >
>
>
> I would actually say that almost nothing should depend on the config file :). 
> I mean this in the sense that the dependencies should be encapsulated 
> similarly to how llvm encapsulates all of the external dependencies and the 
> rest of the code should just use those abstractions.
>
> However, I too have come to the conclusion that the config file could be an 
> exception. My reason for that is that for instance our XML abstraction seems 
> to be misplaced in the Host library. 

[Lldb-commits] [PATCH] D58664: [Utility] Fix ArchSpec.MergeFrom to correctly merge environments

[Alex Langford via Phabricator via lldb-commits]

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

Rebase on master


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

https://reviews.llvm.org/D58664

Files:
  source/Utility/ArchSpec.cpp
  unittests/Utility/ArchSpecTest.cpp


Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -134,22 +134,46 @@
 }
 
 TEST(ArchSpecTest, MergeFrom) {
-  ArchSpec A;
-  ArchSpec B("x86_64-pc-linux");
-
-  EXPECT_FALSE(A.IsValid());
-  ASSERT_TRUE(B.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
-
-  A.MergeFrom(B);
-  ASSERT_TRUE(A.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+  {
+ArchSpec A;
+ArchSpec B("x86_64-pc-linux");
+
+EXPECT_FALSE(A.IsValid());
+ASSERT_TRUE(B.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
+
+A.MergeFrom(B);
+ASSERT_TRUE(A.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+  }
+  {
+ArchSpec A("aarch64");
+ArchSpec B("aarch64--linux-android");
+
+EXPECT_TRUE(A.IsValid());
+EXPECT_TRUE(B.IsValid());
+
+EXPECT_EQ(llvm::Triple::ArchType::aarch64, 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::Android,
+  B.GetTriple().getEnvironment());
+
+A.MergeFrom(B);
+EXPECT_EQ(llvm::Triple::ArchType::aarch64, A.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(llvm::Triple::EnvironmentType::Android,
+  A.GetTriple().getEnvironment());
+  }
 }
 
 TEST(ArchSpecTest, MergeFromMachOUnknown) {
Index: source/Utility/ArchSpec.cpp
===
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -903,9 +903,8 @@
   UpdateCore();
   }
   if (!TripleEnvironmentWasSpecified() &&
-  other.TripleEnvironmentWasSpecified() && !TripleVendorWasSpecified()) {
-if (other.TripleVendorWasSpecified())
-  GetTriple().setEnvironment(other.GetTriple().getEnvironment());
+  other.TripleEnvironmentWasSpecified()) {
+GetTriple().setEnvironment(other.GetTriple().getEnvironment());
   }
   // If this and other are both arm ArchSpecs and this ArchSpec is a generic
   // "some kind of arm" spec but the other ArchSpec is a specific arm core,


Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -134,22 +134,46 @@
 }
 
 TEST(ArchSpecTest, MergeFrom) {
-  ArchSpec A;
-  ArchSpec B("x86_64-pc-linux");
-
-  EXPECT_FALSE(A.IsValid());
-  ASSERT_TRUE(B.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
-
-  A.MergeFrom(B);
-  ASSERT_TRUE(A.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+  {
+ArchSpec A;
+ArchSpec B("x86_64-pc-linux");
+
+EXPECT_FALSE(A.IsValid());
+ASSERT_TRUE(B.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
+
+A.MergeFrom(B);
+ASSERT_TRUE(A.IsValid());
+

[Lldb-commits] [PATCH] D58653: [Utility] Remove Triple{Environment, OS, Vendor}IsUnspecifiedUnknown from ArchSpec

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB354933: [Utility] Remove 
Triple{Environment,OS,Vendor}IsUnspecifiedUnknown from ArchSpec (authored by 
xiaobai, committed by ).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D58653?vs=188457=188473#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58653

Files:
  include/lldb/Utility/ArchSpec.h
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Utility/ArchSpec.cpp
  unittests/Utility/ArchSpecTest.cpp

Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -232,3 +232,76 @@
   EXPECT_FALSE(ArchSpec());
   EXPECT_TRUE(ArchSpec("x86_64-pc-linux"));
 }
+
+TEST(ArchSpecTest, TripleComponentsWereSpecified) {
+  {
+ArchSpec A("");
+ArchSpec B("-");
+ArchSpec C("--");
+ArchSpec D("---");
+
+ASSERT_FALSE(A.TripleVendorWasSpecified());
+ASSERT_FALSE(A.TripleOSWasSpecified());
+ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_FALSE(B.TripleVendorWasSpecified());
+ASSERT_FALSE(B.TripleOSWasSpecified());
+ASSERT_FALSE(B.TripleEnvironmentWasSpecified());
+
+ASSERT_FALSE(C.TripleVendorWasSpecified());
+ASSERT_FALSE(C.TripleOSWasSpecified());
+ASSERT_FALSE(C.TripleEnvironmentWasSpecified());
+
+ASSERT_FALSE(D.TripleVendorWasSpecified());
+ASSERT_FALSE(D.TripleOSWasSpecified());
+ASSERT_FALSE(D.TripleEnvironmentWasSpecified());
+  }
+  {
+// TODO: llvm::Triple::normalize treats the missing components from these
+// triples as specified unknown components instead of unspecified
+// components. We need to either change the behavior in llvm or work around
+// this in lldb.
+ArchSpec A("armv7");
+ArchSpec B("armv7-");
+ArchSpec C("armv7--");
+ArchSpec D("armv7---");
+
+ASSERT_FALSE(A.TripleVendorWasSpecified());
+ASSERT_FALSE(A.TripleOSWasSpecified());
+ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(B.TripleVendorWasSpecified());
+ASSERT_FALSE(B.TripleOSWasSpecified());
+ASSERT_FALSE(B.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(C.TripleVendorWasSpecified());
+ASSERT_TRUE(C.TripleOSWasSpecified());
+ASSERT_FALSE(C.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(D.TripleVendorWasSpecified());
+ASSERT_TRUE(D.TripleOSWasSpecified());
+ASSERT_TRUE(D.TripleEnvironmentWasSpecified());
+  }
+  {
+ArchSpec A("x86_64-unknown");
+ArchSpec B("powerpc-unknown-linux");
+ArchSpec C("i386-pc-windows-msvc");
+ArchSpec D("aarch64-unknown-linux-android");
+
+ASSERT_TRUE(A.TripleVendorWasSpecified());
+ASSERT_FALSE(A.TripleOSWasSpecified());
+ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(B.TripleVendorWasSpecified());
+ASSERT_TRUE(B.TripleOSWasSpecified());
+ASSERT_FALSE(B.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(C.TripleVendorWasSpecified());
+ASSERT_TRUE(C.TripleOSWasSpecified());
+ASSERT_TRUE(C.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(D.TripleVendorWasSpecified());
+ASSERT_TRUE(D.TripleOSWasSpecified());
+ASSERT_TRUE(D.TripleEnvironmentWasSpecified());
+  }
+}
Index: source/Utility/ArchSpec.cpp
===
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -889,10 +889,9 @@
 }
 
 void ArchSpec::MergeFrom(const ArchSpec ) {
-  if (TripleVendorIsUnspecifiedUnknown() &&
-  !other.TripleVendorIsUnspecifiedUnknown())
+  if (!TripleVendorWasSpecified() && other.TripleVendorWasSpecified())
 GetTriple().setVendor(other.GetTriple().getVendor());
-  if (TripleOSIsUnspecifiedUnknown() && !other.TripleOSIsUnspecifiedUnknown())
+  if (!TripleOSWasSpecified() && other.TripleVendorWasSpecified())
 GetTriple().setOS(other.GetTriple().getOS());
   if (GetTriple().getArch() == llvm::Triple::UnknownArch) {
 GetTriple().setArch(other.GetTriple().getArch());
@@ -903,8 +902,8 @@
 if (other.GetCore() != eCore_uknownMach64)
   UpdateCore();
   }
-  if (GetTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
-  !TripleVendorWasSpecified()) {
+  if (!TripleEnvironmentWasSpecified() &&
+  other.TripleEnvironmentWasSpecified() && !TripleVendorWasSpecified()) {
 if (other.TripleVendorWasSpecified())
   GetTriple().setEnvironment(other.GetTriple().getEnvironment());
   }
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -3309,7 +3309,7 @@
   }
 
   if (CalculateType() == eTypeCoreFile &&
-  m_arch_spec.TripleOSIsUnspecifiedUnknown()) {
+  

[Lldb-commits] [lldb] r354933 - [Utility] Remove Triple{Environment, OS, Vendor}IsUnspecifiedUnknown from ArchSpec

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Tue Feb 26 15:50:19 2019
New Revision: 354933

URL: http://llvm.org/viewvc/llvm-project?rev=354933=rev
Log:
[Utility] Remove Triple{Environment,OS,Vendor}IsUnspecifiedUnknown from ArchSpec

Summary:
These functions should always return the opposite of the
`Triple{Environment,OS,Vendor}WasSpecified` functions. Unspecified unknown is
the same as unspecified, which is why one set of functions should give us what
we want. It's possible to have specified unknown, which is why we can't just
rely on checking the enum values of vendor/os/environment. We must also ensure
that the names of these are empty and not "unknown".

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

Modified:
lldb/trunk/include/lldb/Utility/ArchSpec.h
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/trunk/source/Utility/ArchSpec.cpp
lldb/trunk/unittests/Utility/ArchSpecTest.cpp

Modified: lldb/trunk/include/lldb/Utility/ArchSpec.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/ArchSpec.h?rev=354933=354932=354933=diff
==
--- lldb/trunk/include/lldb/Utility/ArchSpec.h (original)
+++ lldb/trunk/include/lldb/Utility/ArchSpec.h Tue Feb 26 15:50:19 2019
@@ -376,20 +376,10 @@ public:
 return !m_triple.getVendorName().empty();
   }
 
-  bool TripleVendorIsUnspecifiedUnknown() const {
-return m_triple.getVendor() == llvm::Triple::UnknownVendor &&
-   m_triple.getVendorName().empty();
-  }
-
   bool TripleOSWasSpecified() const { return !m_triple.getOSName().empty(); }
 
   bool TripleEnvironmentWasSpecified() const {
-return !m_triple.getEnvironmentName().empty();
-  }
-
-  bool TripleOSIsUnspecifiedUnknown() const {
-return m_triple.getOS() == llvm::Triple::UnknownOS &&
-   m_triple.getOSName().empty();
+return m_triple.hasEnvironment();
   }
 
   //--

Modified: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp?rev=354933=354932=354933=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp Tue Feb 26 
15:50:19 2019
@@ -3309,7 +3309,7 @@ ArchSpec ObjectFileELF::GetArchitecture(
   }
 
   if (CalculateType() == eTypeCoreFile &&
-  m_arch_spec.TripleOSIsUnspecifiedUnknown()) {
+  !m_arch_spec.TripleOSWasSpecified()) {
 // Core files don't have section headers yet they have PT_NOTE program
 // headers that might shed more light on the architecture
 for (const elf::ELFProgramHeader  : ProgramHeaders()) {

Modified: lldb/trunk/source/Utility/ArchSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ArchSpec.cpp?rev=354933=354932=354933=diff
==
--- lldb/trunk/source/Utility/ArchSpec.cpp (original)
+++ lldb/trunk/source/Utility/ArchSpec.cpp Tue Feb 26 15:50:19 2019
@@ -889,10 +889,9 @@ bool ArchSpec::ContainsOnlyArch(const ll
 }
 
 void ArchSpec::MergeFrom(const ArchSpec ) {
-  if (TripleVendorIsUnspecifiedUnknown() &&
-  !other.TripleVendorIsUnspecifiedUnknown())
+  if (!TripleVendorWasSpecified() && other.TripleVendorWasSpecified())
 GetTriple().setVendor(other.GetTriple().getVendor());
-  if (TripleOSIsUnspecifiedUnknown() && !other.TripleOSIsUnspecifiedUnknown())
+  if (!TripleOSWasSpecified() && other.TripleVendorWasSpecified())
 GetTriple().setOS(other.GetTriple().getOS());
   if (GetTriple().getArch() == llvm::Triple::UnknownArch) {
 GetTriple().setArch(other.GetTriple().getArch());
@@ -903,8 +902,8 @@ void ArchSpec::MergeFrom(const ArchSpec
 if (other.GetCore() != eCore_uknownMach64)
   UpdateCore();
   }
-  if (GetTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
-  !TripleVendorWasSpecified()) {
+  if (!TripleEnvironmentWasSpecified() &&
+  other.TripleEnvironmentWasSpecified() && !TripleVendorWasSpecified()) {
 if (other.TripleVendorWasSpecified())
   GetTriple().setEnvironment(other.GetTriple().getEnvironment());
   }

Modified: lldb/trunk/unittests/Utility/ArchSpecTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/ArchSpecTest.cpp?rev=354933=354932=354933=diff
==
--- lldb/trunk/unittests/Utility/ArchSpecTest.cpp (original)
+++ lldb/trunk/unittests/Utility/ArchSpecTest.cpp Tue Feb 26 15:50:19 2019
@@ -232,3 +232,76 @@ TEST(ArchSpecTest, OperatorBool) {
   EXPECT_FALSE(ArchSpec());
   EXPECT_TRUE(ArchSpec("x86_64-pc-linux"));
 }
+
+TEST(ArchSpecTest, TripleComponentsWereSpecified) {
+  {
+ArchSpec A("");
+ArchSpec B("-");
+ArchSpec 

[Lldb-commits] [PATCH] D58699: Adapt the ObjC checker instrumentation to handle objc_msgSend with struct returns

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I chose not to do it that way because I don't think it will ever be the case 
the msgSend_stret will NOT be a struct return convention so the test isn't 
relevant (and is a bit confusing) there.  And we may at some point need to do 
some other work that depends on the flavor of msgSend, so I wanted to keep the 
distinction.

If all we are ever going to care about is the return convention, then we should 
remove the msgSend_types classification and just encode the return type and 
have the Inspect pass over the Super sends.  But that would obscure the fact 
that this code is all about instrumenting some of the objc_msgSend flavors, and 
make it harder to read IMO.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58699



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

[Lldb-commits] [PATCH] D58699: Adapt the ObjC checker instrumentation to handle objc_msgSend with struct returns

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Expression/IRDynamicChecks.cpp:437
   break;
 case eMsgSend_stret:
   target_object = call_inst->getArgOperand(1);

move up above for all architectures?



Comment at: source/Expression/IRDynamicChecks.cpp:438-439
 case eMsgSend_stret:
   target_object = call_inst->getArgOperand(1);
   selector = call_inst->getArgOperand(2);
   break;

remove these two lines after moving eMsgSend_stret up with eMsgSend and 
eMsgSend_fpret cases?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58699



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

[Lldb-commits] [PATCH] D58699: Adapt the ObjC checker instrumentation to handle objc_msgSend with struct returns

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added a reviewer: clayborg.
Herald added subscribers: lldb-commits, teemperor, abidh, kristof.beyls, 
javed.absar.
Herald added a reviewer: serge-sans-paille.
Herald added a project: LLDB.

The objc_object_checker instrumentation inserts a call to the checker function 
before each call to any of the family of objc_msgSend calls.  The checker 
function gets passed the object, and the selector from the msgSend.  These 
arguments are in different places in the original call instruction depending on 
whether the method used the struct return convention or not.  Traditionally, 
objc_msgSend was used for scalar returns and objc_msgSent_stret for struct 
return conventions.  But on arm64, both scalar and struct return calls use 
objc_msgSend, so for struct return methods we were passing the checker the 
wrong object pointer and the expression was crashing in the checker.

However, the llvm::Instruction generated by the JIT knows whether it was used 
with struct return convention or not, so add a check for that to the code that 
inserts the checker.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D58699

Files:
  packages/Python/lldbsuite/test/lang/objc/objc-checker/TestObjCCheckers.py
  packages/Python/lldbsuite/test/lang/objc/objc-checker/main.m
  source/Expression/IRDynamicChecks.cpp


Index: source/Expression/IRDynamicChecks.cpp
===
--- source/Expression/IRDynamicChecks.cpp
+++ source/Expression/IRDynamicChecks.cpp
@@ -424,8 +424,15 @@
 switch (msgSend_types[inst]) {
 case eMsgSend:
 case eMsgSend_fpret:
-  target_object = call_inst->getArgOperand(0);
-  selector = call_inst->getArgOperand(1);
+  // On arm64, clang uses objc_msgSend for scalar and struct return
+  // calls.  The call instruction will record which was used.
+  if (call_inst->hasStructRetAttr()) {
+target_object = call_inst->getArgOperand(1);
+selector = call_inst->getArgOperand(2);
+  } else {
+target_object = call_inst->getArgOperand(0);
+selector = call_inst->getArgOperand(1);
+  }
   break;
 case eMsgSend_stret:
   target_object = call_inst->getArgOperand(1);
Index: packages/Python/lldbsuite/test/lang/objc/objc-checker/main.m
===
--- packages/Python/lldbsuite/test/lang/objc/objc-checker/main.m
+++ packages/Python/lldbsuite/test/lang/objc/objc-checker/main.m
@@ -1,11 +1,19 @@
 #import 
 
+// This should be a big enough struct that it will force
+// the struct return convention:
+typedef struct BigStruct {
+  float a, b, c, d, e, f, g, h, i, j, k, l;
+} BigStruct;
+
+
 @interface Simple : NSObject
 {
   int _value;
 }
 - (int) value;
 - (void) setValue: (int) newValue;
+- (BigStruct) getBigStruct;
 @end
 
 @implementation Simple
@@ -18,6 +26,13 @@
 {
   _value = newValue;
 }
+
+- (BigStruct) getBigStruct
+{
+  BigStruct big_struct = {1.0, 2.0, 3.0, 4.0, 5.0, 6.0,
+  7.0, 8.0, 9.0, 10.0, 11.0, 12.0};
+  return big_struct;
+}
 @end
 
 int main ()
Index: packages/Python/lldbsuite/test/lang/objc/objc-checker/TestObjCCheckers.py
===
--- packages/Python/lldbsuite/test/lang/objc/objc-checker/TestObjCCheckers.py
+++ packages/Python/lldbsuite/test/lang/objc/objc-checker/TestObjCCheckers.py
@@ -18,13 +18,15 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
+NO_DEBUG_INFO_TESTCASE = True
+
 def setUp(self):
 # Call super's setUp().
 TestBase.setUp(self)
 
 # Find the line number to break for main.c.
 self.source_name = 'main.m'
-
+
 @skipUnlessDarwin
 @add_test_categories(['pyapi'])
 def test_objc_checker(self):
@@ -77,3 +79,16 @@
 # Make sure the error is helpful:
 err_string = expr_error.GetCString()
 self.assertTrue("selector" in err_string)
+
+#
+# Check that we correctly insert the checker for an
+# ObjC method with the struct return convention.
+# Getting this wrong would cause us to call the checker
+# with the wrong arguments, and the checker would crash
+# So I'm just checking "expression runs successfully" here:
+#
+expr_value = frame.EvaluateExpression("[my_simple getBigStruct]", 
False)
+expr_error = expr_value.GetError()
+
+self.assertTrue(expr_error.Success())
+


Index: source/Expression/IRDynamicChecks.cpp
===
--- source/Expression/IRDynamicChecks.cpp
+++ source/Expression/IRDynamicChecks.cpp
@@ -424,8 +424,15 @@
 switch (msgSend_types[inst]) {
 case eMsgSend:
 case eMsgSend_fpret:
-  target_object = call_inst->getArgOperand(0);
-  selector = call_inst->getArgOperand(1);
+  // On arm64, clang uses objc_msgSend for scalar and struct return

[Lldb-commits] [PATCH] D58653: [Utility] Remove Triple{Environment, OS, Vendor}IsUnspecifiedUnknown from ArchSpec

[Greg Clayton via Phabricator via lldb-commits]

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

Actually on re-reading the patch, we are detecting if anything wasn't set and 
setting it. Makes sense.


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

https://reviews.llvm.org/D58653



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

[Lldb-commits] [PATCH] D58653: [Utility] Remove Triple{Environment, OS, Vendor}IsUnspecifiedUnknown from ArchSpec

[Greg Clayton via Phabricator via lldb-commits]

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

Still issues from what I see. We need to know if something was specified or not 
and with your change we don't know the difference between the "none" state 
(which is current enum = unknown and string = "unknown") and the just not 
specified case (which is current enum = unknown and string = )


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

https://reviews.llvm.org/D58653



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

[Lldb-commits] [PATCH] D58653: [Utility] Remove Triple{Environment, OS, Vendor}IsUnspecifiedUnknown from ArchSpec

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D58653#1411285 , @aprantl wrote:

> @clayborg I recently ran into a similar issue and I think that perhaps adding 
> explicit `llvm::Triple::Any{Vendor|OS|...}` enumerators to llvm::Triple to 
> make this distinction explicit would be the cleanest solution.


Any is fine, we just need a way to say "no os" or "no vendor" or "no 
environment". The way I am thinking about the way things would be:

If we had a "None":

- "Any" would be "unknown" (specified or unspecified)
- "None" would be the new enum/string

If we add "Any":

- "Any" would be the new enum/string
- "None" would be "unknown" (specified or unspecified)

Seems like less work to add the "None". Otherwise we end up having to change 
all much more triple stuff in LLVM because if we init a triple with "armv7", it 
currently defaults to unknown for os, vendor and env and that would need to 
change. Or all constructors in LLDB would need to change to call a different 
constructor that would force any unspecified parts to become "Any". Thoughts?
Not sure which is easier.


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

https://reviews.llvm.org/D58653



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

[Lldb-commits] [PATCH] D58653: [Utility] Remove Triple{Environment, OS, Vendor}IsUnspecifiedUnknown from ArchSpec

[Alex Langford via Phabricator via lldb-commits]

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

Fix typo


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

https://reviews.llvm.org/D58653

Files:
  include/lldb/Utility/ArchSpec.h
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Utility/ArchSpec.cpp
  unittests/Utility/ArchSpecTest.cpp

Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -232,3 +232,76 @@
   EXPECT_FALSE(ArchSpec());
   EXPECT_TRUE(ArchSpec("x86_64-pc-linux"));
 }
+
+TEST(ArchSpecTest, TripleComponentsWereSpecified) {
+  {
+ArchSpec A("");
+ArchSpec B("-");
+ArchSpec C("--");
+ArchSpec D("---");
+
+ASSERT_FALSE(A.TripleVendorWasSpecified());
+ASSERT_FALSE(A.TripleOSWasSpecified());
+ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_FALSE(B.TripleVendorWasSpecified());
+ASSERT_FALSE(B.TripleOSWasSpecified());
+ASSERT_FALSE(B.TripleEnvironmentWasSpecified());
+
+ASSERT_FALSE(C.TripleVendorWasSpecified());
+ASSERT_FALSE(C.TripleOSWasSpecified());
+ASSERT_FALSE(C.TripleEnvironmentWasSpecified());
+
+ASSERT_FALSE(D.TripleVendorWasSpecified());
+ASSERT_FALSE(D.TripleOSWasSpecified());
+ASSERT_FALSE(D.TripleEnvironmentWasSpecified());
+  }
+  {
+// TODO: llvm::Triple::normalize treats the missing components from these
+// triples as specified unknown components instead of unspecified
+// components. We need to either change the behavior in llvm or work around
+// this in lldb.
+ArchSpec A("armv7");
+ArchSpec B("armv7-");
+ArchSpec C("armv7--");
+ArchSpec D("armv7---");
+
+ASSERT_FALSE(A.TripleVendorWasSpecified());
+ASSERT_FALSE(A.TripleOSWasSpecified());
+ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(B.TripleVendorWasSpecified());
+ASSERT_FALSE(B.TripleOSWasSpecified());
+ASSERT_FALSE(B.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(C.TripleVendorWasSpecified());
+ASSERT_TRUE(C.TripleOSWasSpecified());
+ASSERT_FALSE(C.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(D.TripleVendorWasSpecified());
+ASSERT_TRUE(D.TripleOSWasSpecified());
+ASSERT_TRUE(D.TripleEnvironmentWasSpecified());
+  }
+  {
+ArchSpec A("x86_64-unknown");
+ArchSpec B("powerpc-unknown-linux");
+ArchSpec C("i386-pc-windows-msvc");
+ArchSpec D("aarch64-unknown-linux-android");
+
+ASSERT_TRUE(A.TripleVendorWasSpecified());
+ASSERT_FALSE(A.TripleOSWasSpecified());
+ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(B.TripleVendorWasSpecified());
+ASSERT_TRUE(B.TripleOSWasSpecified());
+ASSERT_FALSE(B.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(C.TripleVendorWasSpecified());
+ASSERT_TRUE(C.TripleOSWasSpecified());
+ASSERT_TRUE(C.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(D.TripleVendorWasSpecified());
+ASSERT_TRUE(D.TripleOSWasSpecified());
+ASSERT_TRUE(D.TripleEnvironmentWasSpecified());
+  }
+}
Index: source/Utility/ArchSpec.cpp
===
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -889,10 +889,9 @@
 }
 
 void ArchSpec::MergeFrom(const ArchSpec ) {
-  if (TripleVendorIsUnspecifiedUnknown() &&
-  !other.TripleVendorIsUnspecifiedUnknown())
+  if (!TripleVendorWasSpecified() && other.TripleVendorWasSpecified())
 GetTriple().setVendor(other.GetTriple().getVendor());
-  if (TripleOSIsUnspecifiedUnknown() && !other.TripleOSIsUnspecifiedUnknown())
+  if (!TripleOSWasSpecified() && other.TripleVendorWasSpecified())
 GetTriple().setOS(other.GetTriple().getOS());
   if (GetTriple().getArch() == llvm::Triple::UnknownArch) {
 GetTriple().setArch(other.GetTriple().getArch());
@@ -903,8 +902,8 @@
 if (other.GetCore() != eCore_uknownMach64)
   UpdateCore();
   }
-  if (GetTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
-  !TripleVendorWasSpecified()) {
+  if (!TripleEnvironmentWasSpecified() &&
+  other.TripleEnvironmentWasSpecified() && !TripleVendorWasSpecified()) {
 if (other.TripleVendorWasSpecified())
   GetTriple().setEnvironment(other.GetTriple().getEnvironment());
   }
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -3309,7 +3309,7 @@
   }
 
   if (CalculateType() == eTypeCoreFile &&
-  m_arch_spec.TripleOSIsUnspecifiedUnknown()) {
+  !m_arch_spec.TripleOSWasSpecified()) {
 // Core files don't have section headers yet they have PT_NOTE program
 // headers that might shed more light on the architecture
 for (const elf::ELFProgramHeader  : ProgramHeaders()) {
Index: include/lldb/Utility/ArchSpec.h

[Lldb-commits] [PATCH] D58653: [Utility] Remove Triple{Environment, OS, Vendor}IsUnspecifiedUnknown from ArchSpec

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

@clayborg I recently ran into a similar issue and I think that perhaps adding 
explicit `llvm::Triple::Any{Vendor|OS|...}` enumerators to llvm::Triple to make 
this distinction explicit would be the cleanest solution.


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

https://reviews.llvm.org/D58653



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

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

[Alex Langford via Phabricator via lldb-commits]

xiaobai updated this revision to Diff 188454.
xiaobai added a comment.
Herald added subscribers: arichardson, emaste.
Herald added a reviewer: espindola.

Add more tests and rethink approach


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

https://reviews.llvm.org/D58653

Files:
  include/lldb/Utility/ArchSpec.h
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Utility/ArchSpec.cpp
  unittests/Utility/ArchSpecTest.cpp

Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -232,3 +232,76 @@
   EXPECT_FALSE(ArchSpec());
   EXPECT_TRUE(ArchSpec("x86_64-pc-linux"));
 }
+
+TEST(ArchSpecTest, TripleComponentsWereSpecified) {
+  {
+ArchSpec A("");
+ArchSpec B("-");
+ArchSpec C("--");
+ArchSpec D("---");
+
+ASSERT_FALSE(A.TripleVendorWasSpecified());
+ASSERT_FALSE(A.TripleOSWasSpecified());
+ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_FALSE(B.TripleVendorWasSpecified());
+ASSERT_FALSE(B.TripleOSWasSpecified());
+ASSERT_FALSE(B.TripleEnvironmentWasSpecified());
+
+ASSERT_FALSE(C.TripleVendorWasSpecified());
+ASSERT_FALSE(C.TripleOSWasSpecified());
+ASSERT_FALSE(C.TripleEnvironmentWasSpecified());
+
+ASSERT_FALSE(D.TripleVendorWasSpecified());
+ASSERT_FALSE(D.TripleOSWasSpecified());
+ASSERT_FALSE(D.TripleEnvironmentWasSpecified());
+  }
+  {
+// TODO: llvm::Triple::normalize treats the missing components from these
+// triples as specified unknown components instead of unspecified
+// components. We need to either change the behavior in llvm or work around
+// this in lldb.
+ArchSpec A("armv7");
+ArchSpec B("armv7-");
+ArchSpec C("armv7--");
+ArchSpec D("armv7---");
+
+ASSERT_FALSE(A.TripleVendorWasSpecified());
+ASSERT_FALSE(A.TripleOSWasSpecified());
+ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(B.TripleVendorWasSpecified());
+ASSERT_FALSE(B.TripleOSWasSpecified());
+ASSERT_FALSE(B.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(C.TripleVendorWasSpecified());
+ASSERT_TRUE(C.TripleOSWasSpecified());
+ASSERT_FALSE(C.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(D.TripleVendorWasSpecified());
+ASSERT_TRUE(D.TripleOSWasSpecified());
+ASSERT_TRUE(D.TripleEnvironmentWasSpecified());
+  }
+  {
+ArchSpec A("x86_64-unknown");
+ArchSpec B("powerpc-unknown-linux");
+ArchSpec C("i386-pc-windows-msvc");
+ArchSpec D("aarch64-unknown-linux-android");
+
+ASSERT_TRUE(A.TripleVendorWasSpecified());
+ASSERT_FALSE(A.TripleOSWasSpecified());
+ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(B.TripleVendorWasSpecified());
+ASSERT_TRUE(B.TripleOSWasSpecified());
+ASSERT_FALSE(B.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(C.TripleVendorWasSpecified());
+ASSERT_TRUE(C.TripleOSWasSpecified());
+ASSERT_TRUE(C.TripleEnvironmentWasSpecified());
+
+ASSERT_TRUE(D.TripleVendorWasSpecified());
+ASSERT_TRUE(D.TripleOSWasSpecified());
+ASSERT_TRUE(D.TripleEnvironmentWasSpecified());
+  }
+}
Index: source/Utility/ArchSpec.cpp
===
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -889,10 +889,9 @@
 }
 
 void ArchSpec::MergeFrom(const ArchSpec ) {
-  if (TripleVendorIsUnspecifiedUnknown() &&
-  !other.TripleVendorIsUnspecifiedUnknown())
+  if (!TripleVendorWasSpecified() && other.TripleVendorWasSpecified())
 GetTriple().setVendor(other.GetTriple().getVendor());
-  if (TripleOSIsUnspecifiedUnknown() && !other.TripleOSIsUnspecifiedUnknown())
+  if (!TripleOSWasSpecified() && other.TripleVendorWasSpecified())
 GetTriple().setOS(other.GetTriple().getOS());
   if (GetTriple().getArch() == llvm::Triple::UnknownArch) {
 GetTriple().setArch(other.GetTriple().getArch());
@@ -903,8 +902,8 @@
 if (other.GetCore() != eCore_uknownMach64)
   UpdateCore();
   }
-  if (GetTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
-  !TripleVendorWasSpecified()) {
+  if (!TripleEnvironmentWasSpecified() &&
+  other.TripleEnvironmentWasSpecified() && !TripleVenderWasSpecified()) {
 if (other.TripleVendorWasSpecified())
   GetTriple().setEnvironment(other.GetTriple().getEnvironment());
   }
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -3309,7 +3309,7 @@
   }
 
   if (CalculateType() == eTypeCoreFile &&
-  m_arch_spec.TripleOSIsUnspecifiedUnknown()) {
+  !m_arch_spec.TripleOSWasSpecified()) {
 // Core files don't have section headers yet they have PT_NOTE program
 // headers that might shed more light on the architecture

[Lldb-commits] [PATCH] D58167: Refactor user/group name resolving code

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 188448.
labath marked an inline comment as done.
labath added a comment.

Move the resolver interface into Utility


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

https://reviews.llvm.org/D58167

Files:
  include/lldb/Host/HostInfoBase.h
  include/lldb/Host/posix/HostInfoPosix.h
  include/lldb/Target/Platform.h
  include/lldb/Target/Process.h
  include/lldb/Target/RemoteAwarePlatform.h
  include/lldb/Utility/UserIDResolver.h
  source/Commands/CommandObjectPlatform.cpp
  source/Host/posix/HostInfoPosix.cpp
  source/Plugins/Platform/Kalimba/PlatformKalimba.h
  source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  source/Target/Platform.cpp
  source/Target/Process.cpp
  source/Target/RemoteAwarePlatform.cpp
  source/Utility/CMakeLists.txt
  source/Utility/UserIDResolver.cpp
  unittests/Target/CMakeLists.txt
  unittests/Target/ProcessInstanceInfoTest.cpp
  unittests/Utility/CMakeLists.txt
  unittests/Utility/UserIDResolverTest.cpp

Index: unittests/Utility/UserIDResolverTest.cpp
===
--- /dev/null
+++ unittests/Utility/UserIDResolverTest.cpp
@@ -0,0 +1,47 @@
+//===-- UserIDResolverTest.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/Utility/UserIDResolver.h"
+#include "gmock/gmock.h"
+
+using namespace lldb_private;
+using namespace testing;
+
+namespace {
+class TestUserIDResolver : public UserIDResolver {
+public:
+  MOCK_METHOD1(DoGetUserName, llvm::Optional(id_t uid));
+  MOCK_METHOD1(DoGetGroupName, llvm::Optional(id_t gid));
+};
+} // namespace
+
+TEST(UserIDResolver, GetUserName) {
+  StrictMock r;
+  llvm::StringRef user47("foo");
+  EXPECT_CALL(r, DoGetUserName(47)).Times(1).WillOnce(Return(user47.str()));
+  EXPECT_CALL(r, DoGetUserName(42)).Times(1).WillOnce(Return(llvm::None));
+
+  // Call functions twice to make sure the caching works.
+  EXPECT_EQ(user47, r.GetUserName(47));
+  EXPECT_EQ(user47, r.GetUserName(47));
+  EXPECT_EQ(llvm::None, r.GetUserName(42));
+  EXPECT_EQ(llvm::None, r.GetUserName(42));
+}
+
+TEST(UserIDResolver, GetGroupName) {
+  StrictMock r;
+  llvm::StringRef group47("foo");
+  EXPECT_CALL(r, DoGetGroupName(47)).Times(1).WillOnce(Return(group47.str()));
+  EXPECT_CALL(r, DoGetGroupName(42)).Times(1).WillOnce(Return(llvm::None));
+
+  // Call functions twice to make sure the caching works.
+  EXPECT_EQ(group47, r.GetGroupName(47));
+  EXPECT_EQ(group47, r.GetGroupName(47));
+  EXPECT_EQ(llvm::None, r.GetGroupName(42));
+  EXPECT_EQ(llvm::None, r.GetGroupName(42));
+}
Index: unittests/Utility/CMakeLists.txt
===
--- unittests/Utility/CMakeLists.txt
+++ unittests/Utility/CMakeLists.txt
@@ -34,6 +34,7 @@
   TimeoutTest.cpp
   TimerTest.cpp
   UriParserTest.cpp
+  UserIDResolverTest.cpp
   UUIDTest.cpp
   VASprintfTest.cpp
   VMRangeTest.cpp
Index: unittests/Target/ProcessInstanceInfoTest.cpp
===
--- /dev/null
+++ unittests/Target/ProcessInstanceInfoTest.cpp
@@ -0,0 +1,75 @@
+//===-- ProcessInstanceInfoTest.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/Process.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+namespace {
+/// A very simple resolver which fails for even ids and returns a simple string
+/// for odd ones.
+class DummyUserIDResolver : public UserIDResolver {
+protected:
+  llvm::Optional DoGetUserName(id_t uid) {
+if (uid % 2)
+  return ("user" + llvm::Twine(uid)).str();
+return llvm::None;
+  }
+
+  llvm::Optional DoGetGroupName(id_t gid) {
+if (gid % 2)
+  return ("group" + llvm::Twine(gid)).str();
+return llvm::None;
+  }
+};
+} // namespace
+
+TEST(ProcessInstanceInfo, Dump) {
+  ProcessInstanceInfo info("a.out", ArchSpec("x86_64-pc-linux"), 47);
+  info.SetUserID(1);
+  info.SetEffectiveUserID(2);
+  info.SetGroupID(3);
+  info.SetEffectiveGroupID(4);
+
+  DummyUserIDResolver resolver;
+  StreamString s;
+  info.Dump(s, resolver);
+  EXPECT_STREQ(R"(pid = 47
+   name = a.out
+   file = a.out
+   arch = x86_64-pc-linux
+uid = 1 (user1)
+gid = 3 (group3)
+   euid = 2 ()
+   egid = 4   

[Lldb-commits] [PATCH] D58167: Refactor user/group name resolving code

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: include/lldb/Host/UserIDResolver.h:9
+
+#ifndef LLDB_HOST_USERIDRESOLVER_H
+#define LLDB_HOST_USERIDRESOLVER_H

zturner wrote:
> I wonder if this class should actually be in Host.  While some specific 
> implementation of it might be host-dependent, the interface itself is not.  I 
> kind of envision at some point in the future having a target that contains 
> all of our core interfaces that someone can include and re-implement small 
> pieces of the debugger without having to bring in the entire thing.  This is 
> also nice from a mocking / unittesting perspective.
> 
> So I think this would be better if it were in Utility (or some other 
> top-level library such as Interfaces)
Yes, I've wondered about that too. I went with Host because that was enough to 
make things work, but I certainly see the case for this being Utility too. I'll 
move it there.


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

https://reviews.llvm.org/D58167



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

[Lldb-commits] [PATCH] D58564: [Reproducers] Add command provider

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D58564#1410729 , @JDevlieghere 
wrote:

> In D58564#1410213 , @labath wrote:
>
> > I am sorry that I won't have much time to review this in the next couple of 
> > weeks, but I don't think this is a good direction here. I don't see how 
> > this will interact with the SB API recorder, specifically with things like 
> > SBCommandInterpreter::HandleCommand, and ::HandleCommandsFromFile. The 
> > thing I would expect to see is that SB recorder captures the input of those 
> > commands (for a somewhat broad interpretation of "capture") during 
> > recording, and then substitute this during replay. That way the 
> > CommandInterpreter class would not need (almost?) any modifications.
>
>
> It’s been a while but wasn’t this exactly what you proposed in the other 
> differential? How would you capture commands that are entered interactively 
> (through RunCommqndInterpreter)?
>
> Anyway, I don’t believe this is a concern. The provider here only capture 
> what’s entered interactively, hence the flag. Replaying the API call should 
> work exactly as expected. I’ll double check later today.


I have to admit I haven't looked at this in detail, but the thing I'm missing 
here is the connection between commands and API calls. If we take 
RunCommandInterpreter, for instance, you can see that the lldb driver invokes 
this function three times. How do you ensure the "right" commands get replayed 
as a part of the API call?

If you look at the driver more closely, you'll see that each call to 
RunCommandInterpreter is preceeded by a call to SetInputFileHandle. I don't 
have this idea fully baked, but the way I'd try to approach this is to have 
each SetInputFileHandle create a new buffer where the commands will be stored 
in. Then as the commands are being processed (in RunCommandInterpreter), they 
would be added into this buffer. Then, when replaying you would know that you 
only should replay the commands from the given buffer.

I'd also probably try to capture these commands at a slightly lower level, 
because I am hoping that this will allow us to get rid of the add_to_reproducer 
flag. Ideally, this should fall out naturally due to the different source the 
commands are coming from -- the commands executed through the `HandleCommand` 
API would be captured at the SB boundary, and the "interactive" commands would 
be captured by whoever invokes the command interpreter in interactive mode.

(I have no idea how easy it is to achieve this, but that's how I'd approach 
this.)


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

https://reviews.llvm.org/D58564



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

[Lldb-commits] [PATCH] D58680: [lldb] [unittests] Use non-empty format string for Timer()

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354922: [lldb] [unittests] Use non-empty format string for 
Timer() (authored by mgorny, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58680?vs=188430=188437#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58680

Files:
  lldb/trunk/unittests/Utility/TimerTest.cpp


Index: lldb/trunk/unittests/Utility/TimerTest.cpp
===
--- lldb/trunk/unittests/Utility/TimerTest.cpp
+++ lldb/trunk/unittests/Utility/TimerTest.cpp
@@ -17,7 +17,7 @@
   Timer::ResetCategoryTimes();
   {
 static Timer::Category tcat("CAT1");
-Timer t(tcat, "");
+Timer t(tcat, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
@@ -32,10 +32,10 @@
   Timer::ResetCategoryTimes();
   {
 static Timer::Category tcat1("CAT1");
-Timer t1(tcat1, "");
+Timer t1(tcat1, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
 // Explicitly testing the same category as above.
-Timer t2(tcat1, "");
+Timer t2(tcat1, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
@@ -52,10 +52,10 @@
   Timer::ResetCategoryTimes();
   {
 static Timer::Category tcat1("CAT1");
-Timer t1(tcat1, "");
+Timer t1(tcat1, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(100));
 static Timer::Category tcat2("CAT2");
-Timer t2(tcat2, "");
+Timer t2(tcat2, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;


Index: lldb/trunk/unittests/Utility/TimerTest.cpp
===
--- lldb/trunk/unittests/Utility/TimerTest.cpp
+++ lldb/trunk/unittests/Utility/TimerTest.cpp
@@ -17,7 +17,7 @@
   Timer::ResetCategoryTimes();
   {
 static Timer::Category tcat("CAT1");
-Timer t(tcat, "");
+Timer t(tcat, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
@@ -32,10 +32,10 @@
   Timer::ResetCategoryTimes();
   {
 static Timer::Category tcat1("CAT1");
-Timer t1(tcat1, "");
+Timer t1(tcat1, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
 // Explicitly testing the same category as above.
-Timer t2(tcat1, "");
+Timer t2(tcat1, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
@@ -52,10 +52,10 @@
   Timer::ResetCategoryTimes();
   {
 static Timer::Category tcat1("CAT1");
-Timer t1(tcat1, "");
+Timer t1(tcat1, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(100));
 static Timer::Category tcat2("CAT2");
-Timer t2(tcat2, "");
+Timer t2(tcat2, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r354922 - [lldb] [unittests] Use non-empty format string for Timer()

[Michal Gorny via lldb-commits]

Author: mgorny
Date: Tue Feb 26 12:14:07 2019
New Revision: 354922

URL: http://llvm.org/viewvc/llvm-project?rev=354922=rev
Log:
[lldb] [unittests] Use non-empty format string for Timer()

Pass dummy '.' as format string for Timer() rather than an empty string,
in order to silence gcc warnings about empty format string
(-Wformat-zero-length).  The actual format string is irrelevant
to the test in question.

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

Modified:
lldb/trunk/unittests/Utility/TimerTest.cpp

Modified: lldb/trunk/unittests/Utility/TimerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/TimerTest.cpp?rev=354922=354921=354922=diff
==
--- lldb/trunk/unittests/Utility/TimerTest.cpp (original)
+++ lldb/trunk/unittests/Utility/TimerTest.cpp Tue Feb 26 12:14:07 2019
@@ -17,7 +17,7 @@ TEST(TimerTest, CategoryTimes) {
   Timer::ResetCategoryTimes();
   {
 static Timer::Category tcat("CAT1");
-Timer t(tcat, "");
+Timer t(tcat, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
@@ -32,10 +32,10 @@ TEST(TimerTest, CategoryTimesNested) {
   Timer::ResetCategoryTimes();
   {
 static Timer::Category tcat1("CAT1");
-Timer t1(tcat1, "");
+Timer t1(tcat1, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
 // Explicitly testing the same category as above.
-Timer t2(tcat1, "");
+Timer t2(tcat1, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
@@ -52,10 +52,10 @@ TEST(TimerTest, CategoryTimes2) {
   Timer::ResetCategoryTimes();
   {
 static Timer::Category tcat1("CAT1");
-Timer t1(tcat1, "");
+Timer t1(tcat1, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(100));
 static Timer::Category tcat2("CAT2");
-Timer t2(tcat2, "");
+Timer t2(tcat2, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;


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

[Lldb-commits] [PATCH] D58680: [lldb] [unittests] Use non-empty format string for Timer()

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

mgorny added a comment.

Heh, and I thought we're (NetBSD) the only ones getting it ;-).


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

https://reviews.llvm.org/D58680



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

[Lldb-commits] [PATCH] D58680: [lldb] [unittests] Use non-empty format string for Timer()

[Pavel Labath via Phabricator via lldb-commits]

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

Thanks for tackling this. I've been annoyed by this warning too, but I haven't 
gotten around to doing anything about it yet. :/


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

https://reviews.llvm.org/D58680



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

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

BTW, what's the reason that you have to have this split now? I understand its 
attractiveness from a Zen perspective, but it seems to me that at the end of 
the day, it doesn't have that much impact on the new code you're about to 
write. It seems to me you could still enforce a clean layering on the new code 
by just saying "only ever include stuff from Host or Utility". The fact that 
Host transitively pulls in the kitchen sink is unfortunate, but I don't think 
it should impact you much. And when we finally clean up Host (which is 
something that we'll do eventually, and I hope not too far from now), then the 
new code will magically stop depending on the whole world without any extra 
effort.

In D58654#1410852 , @zturner wrote:

> In D58654#1410817 , @JDevlieghere 
> wrote:
>
> > I'm not a fan of introducing another library for this. Without a clear 
> > timeline of "fixing" Host it's basically as temporary as anything else in 
> > software development. I also think it's confusing (e,g, new files should go 
> > where?). If you really want a clean slate we should move everything in 
> > System that doesn't depend on anything other than Utility until Host is 
> > empty, but I'm sure everyone agrees that unrealistic and not worth the 
> > churn.
> >
> > For the record I also very much support a Host library instead of having 
> > everything in Utility. That being said, I could see the config file being 
> > special enough (in that everything depends on it) to be the exception.
>


I would actually say that almost nothing should depend on the config file :). I 
mean this in the sense that the dependencies should be encapsulated similarly 
to how llvm encapsulates all of the external dependencies and the rest of the 
code should just use those abstractions.

However, I too have come to the conclusion that the config file could be an 
exception. My reason for that is that for instance our XML abstraction seems to 
be misplaced in the Host library. It's kind of true that whether libxml is 
present is a feature of the host system, but that's a fairly odd way of looking 
at things. Right not I am incubating this idea of creating a "Formats" library 
which would house the minidump parser, gdb-remote protocol support code and 
similar, XML support seems like it could fit nicely there. Having the 
LIBXML_AVAILABLE macro in an easily accessible place would help make that 
happen.

>> It seems like we could drop the dependencies on Core and Symbol by moving 
>> Symbols somewhere else. How much work would there be left after Pavel's 
>> patch?

Yea, symbols is going to be the trickiest part. I've been preparing myself 
(both code-wise and mentally) to tackle that by first removing all the other 
easy obstacles first. I have a sort of an idea on how to handle that, but I 
don't want to go into that here.

I suppose one way to make progress here would be to move Symbols into some 
weird temporary package. Then cleaning up the rest of Host should be easy, and 
the temporary package could disappear as soon as Symbols has been dealt with. 
It might make my job of cleaning it up easier, as I could just move the 
modularized functionality back into Host piece by piece. It shouldn't even 
create additional history churn, as that code will need to be rewritten anyway.


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

https://reviews.llvm.org/D58654



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

[Lldb-commits] [PATCH] D58350: Insert random blocks of python code with swig instead of modify-python-lldb.py

[Zachary Turner via Phabricator via lldb-commits]

zturner accepted this revision.
zturner added a comment.

Also lgtm.  Don't call the commit message "Insert random blocks of python code" 
though.  Hopefully the code we're inserting is not actually random :)


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

https://reviews.llvm.org/D58350



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

[Lldb-commits] [PATCH] D58678: Improve step over performance by not stopping at branches that are function calls and stepping into and them out of each one

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I'm fine with leaving the reporting as is.  This really only happens in fairly 
restricted situations (only hardware breakpoints) and neither way of reporting 
the failure seems much better to me, so we needn't over-polish it.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58678



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

[Lldb-commits] [PATCH] D58350: Insert random blocks of python code with swig instead of modify-python-lldb.py

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

One comment seems to have wandered out of place.




Comment at: scripts/lldb.swig:95
+
+# 
==
+# The modify-python-lldb.py script is responsible for post-processing this 
SWIG-

Should this comment have been deleted?


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

https://reviews.llvm.org/D58350



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

[Lldb-commits] [PATCH] D58350: Insert random blocks of python code with swig instead of modify-python-lldb.py

[Jim Ingham via Phabricator via lldb-commits]

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

LGTM.

I don't remember why this was inserted as separate pass.  Maybe we were still 
early on learning what SWIG could do?  That was a long time ago now...


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

https://reviews.llvm.org/D58350



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

[Lldb-commits] [PATCH] D58610: [lldb] [lit] Set LD_LIBRARY_PATH or alike for Suite tests

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB354920: [lldb] [lit] Set LD_LIBRARY_PATH or alike for 
Suite tests (authored by mgorny, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58610?vs=188399=188431#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58610

Files:
  lit/Suite/lit.cfg
  lit/Suite/lit.site.cfg.in


Index: lit/Suite/lit.site.cfg.in
===
--- lit/Suite/lit.site.cfg.in
+++ lit/Suite/lit.site.cfg.in
@@ -5,6 +5,7 @@
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_shlib_dir = "@SHLIBDIR@"
 config.llvm_build_mode = "@LLVM_BUILD_MODE@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lldb_obj_root = "@LLDB_BINARY_DIR@"
Index: lit/Suite/lit.cfg
===
--- lit/Suite/lit.cfg
+++ lit/Suite/lit.cfg
@@ -3,6 +3,7 @@
 # Configuration file for the 'lit' test runner.
 
 import os
+import platform
 import shlex
 
 import lit.formats
@@ -32,6 +33,28 @@
 'detect_stack_use_after_return=1'
   config.environment['DYLD_INSERT_LIBRARIES'] = runtime
 
+# Shared library build of LLVM may require LD_LIBRARY_PATH or equivalent.
+def find_shlibpath_var():
+  if platform.system() in ['Linux', 'FreeBSD', 'NetBSD', 'SunOS']:
+yield 'LD_LIBRARY_PATH'
+  elif platform.system() == 'Darwin':
+yield 'DYLD_LIBRARY_PATH'
+  elif platform.system() == 'Windows':
+yield 'PATH'
+
+for shlibpath_var in find_shlibpath_var():
+  # In stand-alone build llvm_shlib_dir specifies LLDB's lib directory
+  # while llvm_libs_dir specifies LLVM's lib directory.
+  shlibpath = os.path.pathsep.join(
+(config.llvm_shlib_dir,
+ config.llvm_libs_dir,
+ config.environment.get(shlibpath_var, '')))
+  config.environment[shlibpath_var] = shlibpath
+  break
+else:
+  lit_config.warning("unable to inject shared library path on '{}'"
+ .format(platform.system()))
+
 # Build dotest command.
 dotest_cmd = [config.dotest_path, '-q']
 dotest_cmd.extend(config.dotest_args_str.split(';'))


Index: lit/Suite/lit.site.cfg.in
===
--- lit/Suite/lit.site.cfg.in
+++ lit/Suite/lit.site.cfg.in
@@ -5,6 +5,7 @@
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_shlib_dir = "@SHLIBDIR@"
 config.llvm_build_mode = "@LLVM_BUILD_MODE@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lldb_obj_root = "@LLDB_BINARY_DIR@"
Index: lit/Suite/lit.cfg
===
--- lit/Suite/lit.cfg
+++ lit/Suite/lit.cfg
@@ -3,6 +3,7 @@
 # Configuration file for the 'lit' test runner.
 
 import os
+import platform
 import shlex
 
 import lit.formats
@@ -32,6 +33,28 @@
 'detect_stack_use_after_return=1'
   config.environment['DYLD_INSERT_LIBRARIES'] = runtime
 
+# Shared library build of LLVM may require LD_LIBRARY_PATH or equivalent.
+def find_shlibpath_var():
+  if platform.system() in ['Linux', 'FreeBSD', 'NetBSD', 'SunOS']:
+yield 'LD_LIBRARY_PATH'
+  elif platform.system() == 'Darwin':
+yield 'DYLD_LIBRARY_PATH'
+  elif platform.system() == 'Windows':
+yield 'PATH'
+
+for shlibpath_var in find_shlibpath_var():
+  # In stand-alone build llvm_shlib_dir specifies LLDB's lib directory
+  # while llvm_libs_dir specifies LLVM's lib directory.
+  shlibpath = os.path.pathsep.join(
+(config.llvm_shlib_dir,
+ config.llvm_libs_dir,
+ config.environment.get(shlibpath_var, '')))
+  config.environment[shlibpath_var] = shlibpath
+  break
+else:
+  lit_config.warning("unable to inject shared library path on '{}'"
+ .format(platform.system()))
+
 # Build dotest command.
 dotest_cmd = [config.dotest_path, '-q']
 dotest_cmd.extend(config.dotest_args_str.split(';'))
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58610: [lldb] [lit] Set LD_LIBRARY_PATH or alike for Suite tests

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

mgorny added a comment.

Thank you for the review. When I find some more time, I'll try to convert this 
into utility function somewhere in lit.


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

https://reviews.llvm.org/D58610



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

[Lldb-commits] [lldb] r354920 - [lldb] [lit] Set LD_LIBRARY_PATH or alike for Suite tests

[Michal Gorny via lldb-commits]

Author: mgorny
Date: Tue Feb 26 11:46:29 2019
New Revision: 354920

URL: http://llvm.org/viewvc/llvm-project?rev=354920=rev
Log:
[lldb] [lit] Set LD_LIBRARY_PATH or alike for Suite tests

Set LD_LIBRARY_PATH or local platform's equivalent of it when running
the 'Suite' tests.  This is necessary when running tests inside build
tree with BUILD_SHARED_LIBS enabled, in order to make the LLDB modules
load freshly built LLVM libraries.

The code is copied from clang (test/Unit/lit.cfg).  SHLIBDIR
substitution is added to site-config (already present in top-level LLDB
site-config) to future-proof this into supporting stand-alone builds
with shared LLDB libraries.

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

Modified:
lldb/trunk/lit/Suite/lit.cfg
lldb/trunk/lit/Suite/lit.site.cfg.in

Modified: lldb/trunk/lit/Suite/lit.cfg
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lit.cfg?rev=354920=354919=354920=diff
==
--- lldb/trunk/lit/Suite/lit.cfg (original)
+++ lldb/trunk/lit/Suite/lit.cfg Tue Feb 26 11:46:29 2019
@@ -3,6 +3,7 @@
 # Configuration file for the 'lit' test runner.
 
 import os
+import platform
 import shlex
 
 import lit.formats
@@ -32,6 +33,28 @@ if 'Address' in config.llvm_use_sanitize
 'detect_stack_use_after_return=1'
   config.environment['DYLD_INSERT_LIBRARIES'] = runtime
 
+# Shared library build of LLVM may require LD_LIBRARY_PATH or equivalent.
+def find_shlibpath_var():
+  if platform.system() in ['Linux', 'FreeBSD', 'NetBSD', 'SunOS']:
+yield 'LD_LIBRARY_PATH'
+  elif platform.system() == 'Darwin':
+yield 'DYLD_LIBRARY_PATH'
+  elif platform.system() == 'Windows':
+yield 'PATH'
+
+for shlibpath_var in find_shlibpath_var():
+  # In stand-alone build llvm_shlib_dir specifies LLDB's lib directory
+  # while llvm_libs_dir specifies LLVM's lib directory.
+  shlibpath = os.path.pathsep.join(
+(config.llvm_shlib_dir,
+ config.llvm_libs_dir,
+ config.environment.get(shlibpath_var, '')))
+  config.environment[shlibpath_var] = shlibpath
+  break
+else:
+  lit_config.warning("unable to inject shared library path on '{}'"
+ .format(platform.system()))
+
 # Build dotest command.
 dotest_cmd = [config.dotest_path, '-q']
 dotest_cmd.extend(config.dotest_args_str.split(';'))

Modified: lldb/trunk/lit/Suite/lit.site.cfg.in
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lit.site.cfg.in?rev=354920=354919=354920=diff
==
--- lldb/trunk/lit/Suite/lit.site.cfg.in (original)
+++ lldb/trunk/lit/Suite/lit.site.cfg.in Tue Feb 26 11:46:29 2019
@@ -5,6 +5,7 @@ config.llvm_src_root = "@LLVM_SOURCE_DIR
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_shlib_dir = "@SHLIBDIR@"
 config.llvm_build_mode = "@LLVM_BUILD_MODE@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lldb_obj_root = "@LLDB_BINARY_DIR@"


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

[Lldb-commits] [PATCH] D58680: [lldb] [unittests] Use non-empty format string for Timer()

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

mgorny updated this revision to Diff 188430.
mgorny retitled this revision from "[lldb] [unittests] Silence 
-Wformat-zero-length warnings in UtilityTests" to "[lldb] [unittests] Use 
non-empty format string for Timer()".
mgorny edited the summary of this revision.
mgorny added a comment.

Changed to use '.' instead. Thanks for the suggestion, I like this option more 
too ;-). I have wrongly assumed the tests purposely check whether empty string 
works correctly.


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

https://reviews.llvm.org/D58680

Files:
  lldb/unittests/Utility/TimerTest.cpp


Index: lldb/unittests/Utility/TimerTest.cpp
===
--- lldb/unittests/Utility/TimerTest.cpp
+++ lldb/unittests/Utility/TimerTest.cpp
@@ -17,7 +17,7 @@
   Timer::ResetCategoryTimes();
   {
 static Timer::Category tcat("CAT1");
-Timer t(tcat, "");
+Timer t(tcat, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
@@ -32,10 +32,10 @@
   Timer::ResetCategoryTimes();
   {
 static Timer::Category tcat1("CAT1");
-Timer t1(tcat1, "");
+Timer t1(tcat1, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
 // Explicitly testing the same category as above.
-Timer t2(tcat1, "");
+Timer t2(tcat1, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
@@ -52,10 +52,10 @@
   Timer::ResetCategoryTimes();
   {
 static Timer::Category tcat1("CAT1");
-Timer t1(tcat1, "");
+Timer t1(tcat1, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(100));
 static Timer::Category tcat2("CAT2");
-Timer t2(tcat2, "");
+Timer t2(tcat2, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;


Index: lldb/unittests/Utility/TimerTest.cpp
===
--- lldb/unittests/Utility/TimerTest.cpp
+++ lldb/unittests/Utility/TimerTest.cpp
@@ -17,7 +17,7 @@
   Timer::ResetCategoryTimes();
   {
 static Timer::Category tcat("CAT1");
-Timer t(tcat, "");
+Timer t(tcat, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
@@ -32,10 +32,10 @@
   Timer::ResetCategoryTimes();
   {
 static Timer::Category tcat1("CAT1");
-Timer t1(tcat1, "");
+Timer t1(tcat1, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
 // Explicitly testing the same category as above.
-Timer t2(tcat1, "");
+Timer t2(tcat1, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
@@ -52,10 +52,10 @@
   Timer::ResetCategoryTimes();
   {
 static Timer::Category tcat1("CAT1");
-Timer t1(tcat1, "");
+Timer t1(tcat1, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(100));
 static Timer::Category tcat2("CAT2");
-Timer t2(tcat2, "");
+Timer t2(tcat2, ".");
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58678: Improve step over performance by not stopping at branches that are function calls and stepping into and them out of each one

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D58678#1410970 , @jingham wrote:

> I have two questions about this patch.
>
> 1. I want some llvm expert to weigh in on whether
>
>   m_instructions[i]->IsCall()
>
>   always means it returns to the next instruction after the call.  That seems 
> obvious, but since this patch depends on that being true, I'd like to know 
> that it is guaranteed.


Yes, it would be good to know this

> 
> 
> 2. The reason the test had to change (see Jonas' question) is because before 
> we would step over the breakpoint we were stopped at, then try to get to set 
> the next breakpoint, and when that fails we report the stopped step (since 
> the pc has moved) by presenting our usual stop notification.  But the way the 
> branch search goes here, we fail before we do the step-over, and so the PC 
> hasn't moved, and so we don't do the stop listing.
> 
>   I'm not sure whether it is more confusing to get a stop notification when 
> the PC hasn't moved (albeit with an appropriate error) or whether it's more 
> confusing to have two ways the step error could be reported.

I don't think there ever was a stop listing... This the "process status" that 
used to be in there! I believe the step would fail, and it wouldn't print 
anything, yet the PC had actually changed. If we did get a stop listing, then 
no "process status" would be needed? So I view this as an improvement over not 
getting anything and also the "thread step-over" used to claim it succeeded 
even though it failed. The only notification of this was from some tidbit in 
process status where the thread plan explanation claimed it failed.

> This is a pretty minor issue and I really can't come down hard one way or the 
> other...  If others don't have a strong opinion, its probably fine as is.

So seems like there is another patch that might be better done by the stepping 
experts to clean up the "thread stepXXX" inconsistencies in stepping when 
errors happen during a step? I don't currently know enough about how and where 
this would best be done.

Let me know what you think


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58678



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

[Lldb-commits] [PATCH] D58680: [lldb] [unittests] Silence -Wformat-zero-length warnings in UtilityTests

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Honestly, I think I'd just change the tests to use a non-zero format string. 
It's a silly warning, but it probably not worth bloating our cmake code because 
of it. I mean, it's not like the tests actually *have* to have an empty string. 
The string is just empty because there wasn't anything useful to put there.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58680



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

[Lldb-commits] [lldb] r354914 - Mention predicting exception catch at throw site

[Jim Ingham via lldb-commits]

Author: jingham
Date: Tue Feb 26 11:18:09 2019
New Revision: 354914

URL: http://llvm.org/viewvc/llvm-project?rev=354914=rev
Log:
Mention predicting exception catch at throw site

Modified:
lldb/trunk/docs/status/projects.rst
lldb/trunk/www/projects.html

Modified: lldb/trunk/docs/status/projects.rst
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/status/projects.rst?rev=354914=354913=354914=diff
==
--- lldb/trunk/docs/status/projects.rst (original)
+++ lldb/trunk/docs/status/projects.rst Tue Feb 26 11:18:09 2019
@@ -394,3 +394,24 @@ remote memory and its function calling s
 useful to unify these and make the IR interpreter -- both for LLVM and LLDB --
 better. An alternate strategy would be simply to JIT into the current process
 but have callbacks for non-stack memory access.
+
+Teach lldb to predict exception propagation at the throw site
+-
+
+There are a bunch of places in lldb where we need to know at the point where an
+exception is thrown, what frame will catch the exception.  
+
+For instance, if an expression throws an exception, we need to know whether 
the 
+exception will be caught in the course of the expression evaluation.  If so it 
+would be safe to let the expression continue.  But since we would destroy the 
+state of the thread if we let the exception escape the expression, we 
currently 
+stop the expression evaluation if we see a throw.  If we knew where it would be
+caught we could distinguish these two cases.
+
+Similarly, when you step over a call that throws, you want to stop at the 
throw 
+point if you know the exception will unwind past the frame you were stepping 
in,
+but it would annoying to have the step abort every time an exception was 
thrown.
+If we could predict the catching frame, we could do this right.
+
+And of course, this would be a useful piece of information to display when 
stopped 
+at a throw point.

Modified: lldb/trunk/www/projects.html
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/www/projects.html?rev=354914=354913=354914=diff
==
--- lldb/trunk/www/projects.html (original)
+++ lldb/trunk/www/projects.html Tue Feb 26 11:18:09 2019
@@ -467,6 +467,32 @@
 access.
   
 
+
+  Teach lldb to predict exception 
propagation at the throw site
+
+  
+There are a bunch of places in 
lldb where we need to know at the point where an
+exception is thrown, what 
frame will catch the exception.
+  
+  
+  For instance, if an expression 
throws an exception, we need to know whether the
+  exception will be caught in the 
course of the expression evaluation.  If so it
+  would be safe to let the 
expression continue.  But since we would destroy the
+  state of the thread if we let 
the exception escape the expression, we currently
+  stop the expression evaluation 
if we see a throw.  If we knew where it would be
+  caught we could distinguish 
these two cases.
+  
+  
+  Similarly, when you step over a 
call that throws, you want to stop at the throw 
+  point if you know the exception 
will unwind past the frame you were stepping in,
+  but it would annoying to have 
the step abort every time an exception was thrown.
+  If we could predict the catching 
frame, we could do this right.
+  
+  
+And of course, this would be a 
useful piece of information to display when stopped
+at a throw point.
+  
+
   
 



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

[Lldb-commits] [PATCH] D58678: Improve step over performance by not stopping at branches that are function calls and stepping into and them out of each one

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I have two questions about this patch.

1. I want some llvm expert to weigh in on whether

m_instructions[i]->IsCall()

always means it returns to the next instruction after the call.  That seems 
obvious, but since this patch depends on that being true, I'd like to know that 
it is guaranteed.

2. The reason the test had to change (see Jonas' question) is because before we 
would step over the breakpoint we were stopped at, then try to get to set the 
next breakpoint, and when that fails we report the stopped step (since the pc 
has moved) by presenting our usual stop notification.  But the way the branch 
search goes here, we fail before we do the step-over, and so the PC hasn't 
moved, and so we don't do the stop listing.

I'm not sure whether it is more confusing to get a stop notification when the 
PC hasn't moved (albeit with an appropriate error) or whether it's more 
confusing to have two ways the step error could be reported.

This is a pretty minor issue and I really can't come down hard one way or the 
other...  If others don't have a strong opinion, its probably fine as is.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58678



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

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I would be all for adding a "none" as a valid vendor, os or environment to the 
llvm triple class. Then all of the unspecified unknown stuff goes away. But I 
am not sure how likely the LLVM community will like this change since it won't 
be useful for non LLDB code.


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

https://reviews.llvm.org/D58653



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

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D58653#1410907 , @xiaobai wrote:

> In D58653#1410593 , @clayborg wrote:
>
> > A "specified unknown" is when the user actually typed "unknown" in their 
> > triple. IIRC there is no "none" to specify that there is no OS, so we use 
> > "specified unknowns" for this case. In this case we expect the enum _and_ 
> > the string accessor value for vendor and/or OS to be the string "unknown".
>
>
> This will work when the user types "armv7" as their triple, but when they 
> type "armv7--linux" the vendor gets implicitly set to "unknown" even though 
> the user didn't specify it.
>
> In D58653#1410184 , @labath wrote:
>
> > I think this basically defeats the purpose of the `IsUnspecifiedUnknown` 
> > functions, which was to differentiate between foo-unknown-bar and foo--bar 
> > triples. That in itself was a pretty big hack, but it seems that there is 
> > functionality which depends on this. There was a discussion about this back 
> > in december 
> > http://lists.llvm.org/pipermail/lldb-dev/2018-December/014437.html, and 
> > IIRC it converged to some sort of a conclusion, but I don't think there 
> > hasn't been any effort to try to implement it. I'm not sure what exactly it 
> > means about the future of this patch, but I'd be cautious about it.
>
>
> Part of what you said is inconsistent with my understanding. Specifically, if 
> you specify a Triple with the string "foo-unknown-bar" and another Triple 
> with the string "foo--bar" you end up with the same triple. We rely on the 
> `llvm::Triple::normalize` function to create triples from strings, and there 
> is no way to indicate using this method that a triple has unspecified 
> non-optional parts. We are relying on ArchSpec being created with a Triple 
> object and not a StringRef for this behavior to work.


The "llvm::Triple::normalize" must have been added at some point and I missed 
it. That will mess up some assumptions in LLDB that have been around for a 
while.

> Yeah it does kind of defeat the purpose of some of these functions. I would 
> like to change it to just `IsUnknown` (or just delete the functions entirely) 
> because `IsUnspecifiedUnknown`should always be the opposite of the 
> `WasSpecified` functions. My understanding is that the llvm Triple class 
> tries its best to make sure things are marked as the Unknown enum value if 
> they are unspecified or marked as unknown, but there could be some strange 
> edge case I'm not aware of.
> 
> Regardless, I would like to add these tests (or similar ones) to document the 
> understanding of when vendors, OSes, and environments are considered 
> unspecified or not.

I agree testing is good. The main things I would like to stay consistent are:

- if any part of a triple isn't specified, we need to be able to tell so that 
MergeFrom can merge in any unspecified parts (arch, env, os, vendor)
- if any part of a triple was specified, including "unknown" (since there is no 
"none" for OS or vendor), then it should remain as is during a MergeFrom

Target triples are used by many plug-ins to determine if they should load or 
not. The DynamicLoaderStatic is used for bare board development and relies on 
someone being able to specify "armv7-unknown-unknown" so that it knows the 
triple is has a specified "unknown" for OS and vendor. Then it loads all images 
at the address that they are in the file (load address == file address).


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

https://reviews.llvm.org/D58653



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

[Lldb-commits] [PATCH] D58664: [Utility] Fix ArchSpec.MergeFrom to correctly merge environments

[Alex Langford via Phabricator via lldb-commits]

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

More context + rebase on top of master


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

https://reviews.llvm.org/D58664

Files:
  source/Utility/ArchSpec.cpp
  unittests/Utility/ArchSpecTest.cpp


Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -134,22 +134,46 @@
 }
 
 TEST(ArchSpecTest, MergeFrom) {
-  ArchSpec A;
-  ArchSpec B("x86_64-pc-linux");
-
-  EXPECT_FALSE(A.IsValid());
-  ASSERT_TRUE(B.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
-
-  A.MergeFrom(B);
-  ASSERT_TRUE(A.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+  {
+ArchSpec A;
+ArchSpec B("x86_64-pc-linux");
+
+EXPECT_FALSE(A.IsValid());
+ASSERT_TRUE(B.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
+
+A.MergeFrom(B);
+ASSERT_TRUE(A.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+  }
+  {
+ArchSpec A("aarch64");
+ArchSpec B("aarch64--linux-android");
+
+EXPECT_TRUE(A.IsValid());
+EXPECT_TRUE(B.IsValid());
+
+EXPECT_EQ(llvm::Triple::ArchType::aarch64, 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::Android,
+  B.GetTriple().getEnvironment());
+
+A.MergeFrom(B);
+EXPECT_EQ(llvm::Triple::ArchType::aarch64, A.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(llvm::Triple::EnvironmentType::Android,
+  A.GetTriple().getEnvironment());
+  }
 }
 
 TEST(ArchSpecTest, MergeFromMachOUnknown) {
Index: source/Utility/ArchSpec.cpp
===
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -903,10 +903,8 @@
 if (other.GetCore() != eCore_uknownMach64)
   UpdateCore();
   }
-  if (GetTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
-  !TripleVendorWasSpecified()) {
-if (other.TripleVendorWasSpecified())
-  GetTriple().setEnvironment(other.GetTriple().getEnvironment());
+  if (GetTriple().getEnvironment() == llvm::Triple::UnknownEnvironment) {
+GetTriple().setEnvironment(other.GetTriple().getEnvironment());
   }
   // If this and other are both arm ArchSpecs and this ArchSpec is a generic
   // "some kind of arm" spec but the other ArchSpec is a specific arm core,


Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -134,22 +134,46 @@
 }
 
 TEST(ArchSpecTest, MergeFrom) {
-  ArchSpec A;
-  ArchSpec B("x86_64-pc-linux");
-
-  EXPECT_FALSE(A.IsValid());
-  ASSERT_TRUE(B.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
-
-  A.MergeFrom(B);
-  ASSERT_TRUE(A.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+  {
+ArchSpec A;
+ArchSpec B("x86_64-pc-linux");
+
+EXPECT_FALSE(A.IsValid());
+ASSERT_TRUE(B.IsValid());
+EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());

[Lldb-commits] [PATCH] D58678: Improve step over performance by not stopping at branches that are function calls and stepping into and them out of each one

[Greg Clayton via Phabricator via lldb-commits]

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



Comment at: include/lldb/Core/Disassembler.h:309
+  /// It true, then fine the first branch instruction that isn't
+  /// a function call (a branch that calls and returns to the next
+  /// instruction). If false, find the instruction index of any 

JDevlieghere wrote:
> s/fine/find/
I'll fix that



Comment at: 
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py:86
 substrs=[
-'step over failed',
-'Could not create hardware breakpoint for thread plan'
+'error: Could not create hardware breakpoint for thread plan.'
 ])

JDevlieghere wrote:
> Why did you remove the 'step over failed' substring?
After this change the step doesn't occur because it fails to set the hardware 
breakpoint, so the UI doesn't update and we don't need the process status. 

Before this change, the step was actually incorrectly single stepping into the 
function, then realizing it can't set the hardware breakpoint that was needed 
in order to step back out of the fucntion and the step was aborted after 
partially starting it. The "thread step-over" would also incorrectly return 
success (as we can see from the:
```
self.expect("thread step-over")
```
This line requires the command returns "success" unless you pass "error=True".

Now it just doesn't do the step at all and the error is returned form the 
"thread step-over". 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58678



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

[Lldb-commits] [PATCH] D58664: [Utility] Fix ArchSpec.MergeFrom to correctly merge environments

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D58664#1410750 , @aprantl wrote:

> Could you please add more context to the diff? Something like `git diff 
> -U HEAD~1` works fine.


Will do. It looks like the dependent diff might change significantly so I'm 
going to rebase this on top of master instead since the functionality isn't 
technically tied to the dependent diff.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58664



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

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D58653#1410593 , @clayborg wrote:

> A "specified unknown" is when the user actually typed "unknown" in their 
> triple. IIRC there is no "none" to specify that there is no OS, so we use 
> "specified unknowns" for this case. In this case we expect the enum _and_ the 
> string accessor value for vendor and/or OS to be the string "unknown".


This will work when the user types "armv7" as their triple, but when they type 
"armv7--linux" the vendor gets implicitly set to "unknown" even though the user 
didn't specify it.

In D58653#1410184 , @labath wrote:

> I think this basically defeats the purpose of the `IsUnspecifiedUnknown` 
> functions, which was to differentiate between foo-unknown-bar and foo--bar 
> triples. That in itself was a pretty big hack, but it seems that there is 
> functionality which depends on this. There was a discussion about this back 
> in december 
> http://lists.llvm.org/pipermail/lldb-dev/2018-December/014437.html, and IIRC 
> it converged to some sort of a conclusion, but I don't think there hasn't 
> been any effort to try to implement it. I'm not sure what exactly it means 
> about the future of this patch, but I'd be cautious about it.


Part of what you said is inconsistent with my understanding. Specifically, if 
you specify a Triple with the string "foo-unknown-bar" and another Triple with 
the string "foo--bar" you end up with the same triple. We rely on the 
`llvm::Triple::normalize` function to create triples from strings, and there is 
no way to indicate using this method that a triple has unspecified non-optional 
parts. We are relying on ArchSpec being created with a Triple object and not a 
StringRef for this behavior to work.

Yeah it does kind of defeat the purpose of some of these functions. I would 
like to change it to just `IsUnknown` (or just delete the functions entirely) 
because `IsUnspecifiedUnknown`should always be the opposite of the 
`WasSpecified` functions. My understanding is that the llvm Triple class tries 
its best to make sure things are marked as the Unknown enum value if they are 
unspecified or marked as unknown, but there could be some strange edge case I'm 
not aware of.

Regardless, I would like to add these tests (or similar ones) to document the 
understanding of when vendors, OSes, and environments are considered 
unspecified or not.


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

https://reviews.llvm.org/D58653



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

[Lldb-commits] [PATCH] D58678: Improve step over performance by not stopping at branches that are function calls and stepping into and them out of each one

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D58678#1410861 , @zturner wrote:

> > Since we are stepping over we can safely ignore these calls since they will 
> > return to the next instruction
>
> What if the call throws an exception?


This patch won't change lldb's behavior when an exception is thrown.  Before 
the patch we would step in to the function, set a breakpoint on the return 
instruction and continue.  With this patch, we will continue over the call w/o 
the step in part.  In neither case would we catch a thrown exception.

To deal with thrown exceptions correctly you really need to be able to predict 
where the exception will be caught.  If a step over steps over an exception 
throw that is caught below the frame in which you are stepping, you don't want 
to do anything.  But if the exception is caught in an older frame than the 
stepping frame, you should probably stop.  But LLDB doesn't know how to analyze 
the throw mechanism at the throw site however, so we don't do anything smart 
here.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58678



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

[Lldb-commits] [PATCH] D58678: Improve step over performance by not stopping at branches that are function calls and stepping into and them out of each one

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

> Since we are stepping over we can safely ignore these calls since they will 
> return to the next instruction

What if the call throws an exception?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58678



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

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In D58654#1410817 , @JDevlieghere 
wrote:

> I'm not a fan of introducing another library for this. Without a clear 
> timeline of "fixing" Host it's basically as temporary as anything else in 
> software development. I also think it's confusing (e,g, new files should go 
> where?). If you really want a clean slate we should move everything in System 
> that doesn't depend on anything other than Utility until Host is empty, but 
> I'm sure everyone agrees that unrealistic and not worth the churn.
>
> For the record I also very much support a Host library instead of having 
> everything in Utility. That being said, I could see the config file being 
> special enough (in that everything depends on it) to be the exception.
>
> It seems like we could drop the dependencies on Core and Symbol by moving 
> Symbols somewhere else. How much work would there be left after Pavel's patch?


Moving Symbols somewhere else isn't exactly easy though.  There's a lot of 
interdependencies that are slightly different on every Host platform, so it 
creates a lot of breakage doing things this way.

FWIW, I actually wouldn't mind moving everything out of Host into System until 
Host is basically empty.  My next step after this patch was going to be to move 
MainLoop followed by Socket into System (those are my actual goals anyway).  In 
some ways, approaching the dependency problem from both directions this way is 
more effective than approaching it from only one side, as this way eventually 
Host becomes so small that you can just take whatever is left over and have it 
be subsumed by some other target (probably Core or Symbols).

All of that said, I'm not as sold that having a separate library just for Host 
is even terribly useful.  I'll go along with it if that's what everyone wants, 
but at the end of the day, this library (whether it be called Host, or System, 
or something else) will almost certainly depend on Utility, and Utility will 
almost certainly depend on it (for the same reason that Utility currently 
depends on llvm/Support).  This is why in LLVM/Support there are both non-Host 
specific things, such as `StringExtras.h`, and host specific things, such as 
`FileSystem.h`, `Thread.h`, etc.  I don't think that's actually so bad in 
practice.

So my vote is honestly still to just put this in Utility for consistency with 
LLVM, but I can agree with `System` or something as part of a transition if it 
gets us closer to being able to use these utility classes without a dependency 
on the whole debugger.


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

https://reviews.llvm.org/D58654



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

[Lldb-commits] [PATCH] D58678: Improve step over performance by not stopping at branches that are function calls and stepping into and them out of each one

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: include/lldb/Core/Disassembler.h:309
+  /// It true, then fine the first branch instruction that isn't
+  /// a function call (a branch that calls and returns to the next
+  /// instruction). If false, find the instruction index of any 

s/fine/find/



Comment at: 
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py:86
 substrs=[
-'step over failed',
-'Could not create hardware breakpoint for thread plan'
+'error: Could not create hardware breakpoint for thread plan.'
 ])

Why did you remove the 'step over failed' substring?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58678



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

[Lldb-commits] [PATCH] D58610: [lldb] [lit] Set LD_LIBRARY_PATH or alike for Suite tests

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

mgorny updated this revision to Diff 188399.
mgorny edited the summary of this revision.
mgorny added a comment.

While working on other patches, I've noticed top-level lit.site.cfg* has 
SHLIBDIR which serves the same purpose as in clang. I've updated this patch to 
include it for Suite tests, and therefore future-proof it for stand-alone 
builds with shared LLDB libraries. This is also what clang does, exactly.

I've also fixed indentation to match LLDB style.


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

https://reviews.llvm.org/D58610

Files:
  lldb/lit/Suite/lit.cfg
  lldb/lit/Suite/lit.site.cfg.in


Index: lldb/lit/Suite/lit.site.cfg.in
===
--- lldb/lit/Suite/lit.site.cfg.in
+++ lldb/lit/Suite/lit.site.cfg.in
@@ -5,6 +5,7 @@
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_shlib_dir = "@SHLIBDIR@"
 config.llvm_build_mode = "@LLVM_BUILD_MODE@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lldb_obj_root = "@LLDB_BINARY_DIR@"
Index: lldb/lit/Suite/lit.cfg
===
--- lldb/lit/Suite/lit.cfg
+++ lldb/lit/Suite/lit.cfg
@@ -3,6 +3,7 @@
 # Configuration file for the 'lit' test runner.
 
 import os
+import platform
 import shlex
 
 import lit.formats
@@ -32,6 +33,28 @@
 'detect_stack_use_after_return=1'
   config.environment['DYLD_INSERT_LIBRARIES'] = runtime
 
+# Shared library build of LLVM may require LD_LIBRARY_PATH or equivalent.
+def find_shlibpath_var():
+  if platform.system() in ['Linux', 'FreeBSD', 'NetBSD', 'SunOS']:
+yield 'LD_LIBRARY_PATH'
+  elif platform.system() == 'Darwin':
+yield 'DYLD_LIBRARY_PATH'
+  elif platform.system() == 'Windows':
+yield 'PATH'
+
+for shlibpath_var in find_shlibpath_var():
+  # In stand-alone build llvm_shlib_dir specifies LLDB's lib directory
+  # while llvm_libs_dir specifies LLVM's lib directory.
+  shlibpath = os.path.pathsep.join(
+(config.llvm_shlib_dir,
+ config.llvm_libs_dir,
+ config.environment.get(shlibpath_var, '')))
+  config.environment[shlibpath_var] = shlibpath
+  break
+else:
+  lit_config.warning("unable to inject shared library path on '{}'"
+ .format(platform.system()))
+
 # Build dotest command.
 dotest_cmd = [config.dotest_path, '-q']
 dotest_cmd.extend(config.dotest_args_str.split(';'))


Index: lldb/lit/Suite/lit.site.cfg.in
===
--- lldb/lit/Suite/lit.site.cfg.in
+++ lldb/lit/Suite/lit.site.cfg.in
@@ -5,6 +5,7 @@
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_shlib_dir = "@SHLIBDIR@"
 config.llvm_build_mode = "@LLVM_BUILD_MODE@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lldb_obj_root = "@LLDB_BINARY_DIR@"
Index: lldb/lit/Suite/lit.cfg
===
--- lldb/lit/Suite/lit.cfg
+++ lldb/lit/Suite/lit.cfg
@@ -3,6 +3,7 @@
 # Configuration file for the 'lit' test runner.
 
 import os
+import platform
 import shlex
 
 import lit.formats
@@ -32,6 +33,28 @@
 'detect_stack_use_after_return=1'
   config.environment['DYLD_INSERT_LIBRARIES'] = runtime
 
+# Shared library build of LLVM may require LD_LIBRARY_PATH or equivalent.
+def find_shlibpath_var():
+  if platform.system() in ['Linux', 'FreeBSD', 'NetBSD', 'SunOS']:
+yield 'LD_LIBRARY_PATH'
+  elif platform.system() == 'Darwin':
+yield 'DYLD_LIBRARY_PATH'
+  elif platform.system() == 'Windows':
+yield 'PATH'
+
+for shlibpath_var in find_shlibpath_var():
+  # In stand-alone build llvm_shlib_dir specifies LLDB's lib directory
+  # while llvm_libs_dir specifies LLVM's lib directory.
+  shlibpath = os.path.pathsep.join(
+(config.llvm_shlib_dir,
+ config.llvm_libs_dir,
+ config.environment.get(shlibpath_var, '')))
+  config.environment[shlibpath_var] = shlibpath
+  break
+else:
+  lit_config.warning("unable to inject shared library path on '{}'"
+ .format(platform.system()))
+
 # Build dotest command.
 dotest_cmd = [config.dotest_path, '-q']
 dotest_cmd.extend(config.dotest_args_str.split(';'))
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

I'm not a fan of introducing another library for this. Without a clear timeline 
of "fixing" Host it's basically as temporary as anything else in software 
development. I also think it's confusing (e,g, new files should go where?). If 
you really want a clean slate we should move everything in System that doesn't 
depend on anything other than Utility until Host is empty, but I'm sure 
everyone agrees that unrealistic and not worth the churn.

For the record I also very much support a Host library instead of having 
everything in Utility. That being said, I could see the config file being 
special enough (in that everything depends on it) to be the exception.

It seems like we could drop the dependencies on Core and Symbol by moving 
Symbols somewhere else. How much work would there be left after Pavel's patch?


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

https://reviews.llvm.org/D58654



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

[Lldb-commits] [PATCH] D58167: Refactor user/group name resolving code

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: include/lldb/Host/UserIDResolver.h:9
+
+#ifndef LLDB_HOST_USERIDRESOLVER_H
+#define LLDB_HOST_USERIDRESOLVER_H

I wonder if this class should actually be in Host.  While some specific 
implementation of it might be host-dependent, the interface itself is not.  I 
kind of envision at some point in the future having a target that contains all 
of our core interfaces that someone can include and re-implement small pieces 
of the debugger without having to bring in the entire thing.  This is also nice 
from a mocking / unittesting perspective.

So I think this would be better if it were in Utility (or some other top-level 
library such as Interfaces)


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

https://reviews.llvm.org/D58167



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

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

[Zachary Turner via Phabricator via lldb-commits]

zturner updated this revision to Diff 188393.
zturner added a comment.

Moved to `System` instead of `Utility`


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

https://reviews.llvm.org/D58654

Files:
  lldb/cmake/XcodeHeaderGenerator/CMakeLists.txt
  lldb/cmake/modules/LLDBGenerateConfig.cmake
  lldb/include/lldb/Host/Config.h
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/MainLoop.h
  lldb/include/lldb/Host/Terminal.h
  lldb/include/lldb/Host/linux/Uio.h
  lldb/include/lldb/System/lldb-config.h
  lldb/include/lldb/System/lldb-config.h.cmake
  lldb/include/lldb/Target/Process.h
  lldb/source/Host/common/File.cpp
  lldb/source/Host/common/HostInfoBase.cpp
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Host/common/PseudoTerminal.cpp
  lldb/source/Host/common/Socket.cpp
  lldb/source/Host/common/TCPSocket.cpp
  lldb/source/Host/common/Terminal.cpp
  lldb/source/Host/common/UDPSocket.cpp
  lldb/source/Host/linux/HostInfoLinux.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
  lldb/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
  lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
  lldb/source/Plugins/Platform/OpenBSD/PlatformOpenBSD.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/unittests/Host/SocketTest.cpp

Index: lldb/unittests/Host/SocketTest.cpp
===
--- lldb/unittests/Host/SocketTest.cpp
+++ lldb/unittests/Host/SocketTest.cpp
@@ -12,10 +12,10 @@
 
 #include "gtest/gtest.h"
 
-#include "lldb/Host/Config.h"
 #include "lldb/Host/Socket.h"
 #include "lldb/Host/common/TCPSocket.h"
 #include "lldb/Host/common/UDPSocket.h"
+#include "lldb/System/lldb-config.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#include "lldb/Host/Config.h"
+#include "lldb/System/lldb-config.h"
 
 #include 
 #include 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -19,11 +19,11 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
 
-#include "lldb/Host/Config.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/FileAction.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/System/lldb-config.h"
 #include "lldb/Target/Platform.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/UnixSignals.h"
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -8,7 +8,7 @@
 
 #include 
 
-#include "lldb/Host/Config.h"
+#include "lldb/System/lldb-config.h"
 
 #include "GDBRemoteCommunicationServerLLGS.h"
 #include "lldb/Utility/StreamGDBRemote.h"
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -18,7 +18,6 @@
 #include 
 
 #include "lldb/Core/ModuleSpec.h"
-#include "lldb/Host/Config.h"
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileAction.h"
 #include "lldb/Host/FileSystem.h"
@@ -27,6 +26,7 @@
 #include "lldb/Host/SafeMachO.h"
 #include "lldb/Interpreter/OptionArgParser.h"
 #include 

[Lldb-commits] [PATCH] D58664: [Utility] Fix ArchSpec.MergeFrom to correctly merge environments

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Could you please add more context to the diff? Something like `git diff -U 
HEAD~1` works fine.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58664



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

[Lldb-commits] [PATCH] D58564: [Reproducers] Add command provider

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D58564#1410213 , @labath wrote:

> I am sorry that I won't have much time to review this in the next couple of 
> weeks, but I don't think this is a good direction here. I don't see how this 
> will interact with the SB API recorder, specifically with things like 
> SBCommandInterpreter::HandleCommand, and ::HandleCommandsFromFile. The thing 
> I would expect to see is that SB recorder captures the input of those 
> commands (for a somewhat broad interpretation of "capture") during recording, 
> and then substitute this during replay. That way the CommandInterpreter class 
> would not need (almost?) any modifications.


It’s been a while but wasn’t this exactly what you proposed in the other 
differential? How would you capture commands that are entered interactively 
(through RunCommqndInterpreter)?

Anyway, I don’t believe this is a concern. The provider here only capture 
what’s entered interactively, hence the flag. Replaying the API call should 
work exactly as expected. I’ll double check later today.

> With this approach (shoving all commands into a single stream in the 
> CommandInterpreter) it becomes impossible to replay the API calls above. If 
> you want to proceed with this, then go ahead (it's your feature), but I 
> believe you'll run into some problems down the line.




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

https://reviews.llvm.org/D58564



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

[Lldb-commits] [PATCH] D58680: [lldb] [unittests] Silence -Wformat-zero-length warnings in UtilityTests

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

mgorny created this revision.
mgorny added reviewers: labath, zturner, teemperor.
mgorny added a project: LLDB.
Herald added subscribers: jdoerfert, abidh.

Silence zero-length format warnings in UtilityTests reported by GCC.
Zero-length format strings are permitted by the standards, and are used
purposedly in the tests.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D58680

Files:
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/unittests/Utility/CMakeLists.txt


Index: lldb/unittests/Utility/CMakeLists.txt
===
--- lldb/unittests/Utility/CMakeLists.txt
+++ lldb/unittests/Utility/CMakeLists.txt
@@ -1,3 +1,7 @@
+if(CXX_SUPPORTS_NO_FORMAT_ZERO_LENGTH)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-format-zero-length")
+endif()
+
 add_lldb_unittest(UtilityTests
   AnsiTerminalTest.cpp
   ArgsTest.cpp
Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -278,6 +278,9 @@
 check_cxx_compiler_flag("-Wno-nested-anon-types"
 CXX_SUPPORTS_NO_NESTED_ANON_TYPES)
 
+check_cxx_compiler_flag("-Wno-format-zero-length"
+CXX_SUPPORTS_NO_FORMAT_ZERO_LENGTH)
+
 # Disable MSVC warnings
 if( MSVC )
   add_definitions(


Index: lldb/unittests/Utility/CMakeLists.txt
===
--- lldb/unittests/Utility/CMakeLists.txt
+++ lldb/unittests/Utility/CMakeLists.txt
@@ -1,3 +1,7 @@
+if(CXX_SUPPORTS_NO_FORMAT_ZERO_LENGTH)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-format-zero-length")
+endif()
+
 add_lldb_unittest(UtilityTests
   AnsiTerminalTest.cpp
   ArgsTest.cpp
Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -278,6 +278,9 @@
 check_cxx_compiler_flag("-Wno-nested-anon-types"
 CXX_SUPPORTS_NO_NESTED_ANON_TYPES)
 
+check_cxx_compiler_flag("-Wno-format-zero-length"
+CXX_SUPPORTS_NO_FORMAT_ZERO_LENGTH)
+
 # Disable MSVC warnings
 if( MSVC )
   add_definitions(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58678: Improve step over performance by not stopping at branches that are function calls and stepping into and them out of each one

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added a reviewer: jingham.
Herald added a reviewer: serge-sans-paille.
Herald added a project: LLDB.

Currently when we single step over a source line, we run and stop at every 
branch in the source line range. We can reduce the number of times we stop when 
stepping over by figuring out if any of these branches are function calls, and 
if so, ignore these branches. Since we are stepping over we can safely ignore 
these calls since they will return to the next instruction. Currently the step 
logic would stop at those branches (1st stop), single step into the branch (2nd 
stop), and then set a breakpoint at the return address (3rd stop), and then 
continue.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D58678

Files:
  include/lldb/Core/Disassembler.h
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
  source/Core/Disassembler.cpp
  source/Target/Process.cpp
  source/Target/ThreadPlanStepRange.cpp

Index: source/Target/ThreadPlanStepRange.cpp
===
--- source/Target/ThreadPlanStepRange.cpp
+++ source/Target/ThreadPlanStepRange.cpp
@@ -313,9 +313,10 @@
 return false;
   else {
 Target  = GetThread().GetProcess()->GetTarget();
-uint32_t branch_index;
-branch_index =
-instructions->GetIndexOfNextBranchInstruction(pc_index, target);
+const bool ignore_calls = GetKind() == eKindStepOverRange;
+uint32_t branch_index =
+instructions->GetIndexOfNextBranchInstruction(pc_index, target,
+  ignore_calls);
 
 Address run_to_address;
 
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -6019,7 +6019,8 @@
   }
 
   uint32_t branch_index =
-  insn_list->GetIndexOfNextBranchInstruction(insn_offset, target);
+  insn_list->GetIndexOfNextBranchInstruction(insn_offset, target,
+ false /* ignore_calls*/);
   if (branch_index == UINT32_MAX) {
 return retval;
   }
Index: source/Core/Disassembler.cpp
===
--- source/Core/Disassembler.cpp
+++ source/Core/Disassembler.cpp
@@ -1087,13 +1087,16 @@
 
 uint32_t
 InstructionList::GetIndexOfNextBranchInstruction(uint32_t start,
- Target ) const {
+ Target ,
+ bool ignore_calls) const {
   size_t num_instructions = m_instructions.size();
 
   uint32_t next_branch = UINT32_MAX;
   size_t i;
   for (i = start; i < num_instructions; i++) {
 if (m_instructions[i]->DoesBranch()) {
+  if (ignore_calls && m_instructions[i]->IsCall())
+continue;
   next_branch = i;
   break;
 }
Index: packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
===
--- packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
+++ packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
@@ -79,12 +79,11 @@
 self.runCmd("settings set target.require-hardware-breakpoint true")
 
 # Step over doesn't fail immediately but fails later on.
-self.expect("thread step-over")
 self.expect(
-"process status",
+"thread step-over",
+error=True,
 substrs=[
-'step over failed',
-'Could not create hardware breakpoint for thread plan'
+'error: Could not create hardware breakpoint for thread plan.'
 ])
 
 @skipIfWindows
Index: include/lldb/Core/Disassembler.h
===
--- include/lldb/Core/Disassembler.h
+++ include/lldb/Core/Disassembler.h
@@ -292,8 +292,32 @@
 
   lldb::InstructionSP GetInstructionAtIndex(size_t idx) const;
 
+  //--
+  /// Get the index of the next branch instruction.
+  ///
+  /// Given a list of instructions, find the next branch instruction
+  /// in the list by returning an index.
+  ///
+  /// @param[in] start
+  /// The instruction index of the first instruction to check.
+  ///
+  /// @param[in] target
+  /// A LLDB target object that is used to resolve addresses.
+  ///
+  /// @param[in] ignore_calls
+  /// It true, then fine the first branch instruction that isn't
+  /// a function call (a branch that calls and returns to the next
+  /// instruction). If false, find the instruction index of any 
+  /// branch in the list.
+  ///
+  /// 

Re: [Lldb-commits] [lldb] r354890 - Fix short options syntax in Minidump test

[Davide Italiano via lldb-commits]

Looks like it's green now.

Thanks!

On Tue, Feb 26, 2019 at 8:32 AM Tatyana Krasnukha
 wrote:
>
> This was fixed with r354890 ->  
> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/20544/
> Do you still have the issue?
>
> -Original Message-
> From: Davide Italiano 
> Sent: Tuesday, February 26, 2019 7:28 PM
> To: Tatyana Krasnukha 
> Cc: lldb-commits 
> Subject: Re: [Lldb-commits] [lldb] r354890 - Fix short options syntax in 
> Minidump test
>
> lldb: unrecognized option `--C'
> error: unknown or ambiguous option
> /Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/lldb/lit/Minidump/dump-all.test:55:13:
> error: CHECKCPU: expected string not found in input
>
> this seems to be the issue. Can you give another shot at fixing this or 
> reverting? Thanks!
>
> On Tue, Feb 26, 2019 at 8:16 AM Tatyana Krasnukha 
>  wrote:
> >
> > Those build didn't contain r354890 yet.
> >
> > -Original Message-
> > From: Davide Italiano 
> > Sent: Tuesday, February 26, 2019 7:05 PM
> > To: Tatyana Krasnukha 
> > Cc: lldb-commits 
> > Subject: Re: [Lldb-commits] [lldb] r354890 - Fix short options syntax
> > in Minidump test
> >
> > Link to the failure:
> > https://urldefense.proofpoint.com/v2/url?u=http-3A__green.lab.llvm.org
> > _green_job_lldb-2Dcmake__20543_console=DwIBaQ=DPL6_X_6JkXFx7AXWqB0
> > tg=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw=g6rcurKPvoK7uD68Vi8
> > adZfyKmJS9bRAr63MKkywSXk=jdhvETIBQFdrr0fN0efcvAVZ3EOQdqaYgbWsDZDdEEA
> > =
> >
> > On Tue, Feb 26, 2019 at 8:04 AM Davide Italiano  
> > wrote:
> > >
> > > Tatyana, this commit broke one of the bots:
> > >
> > > Testing Time: 315.65s
> > > 
> > > Failing Tests (1):
> > > LLDB :: Minidump/dump-all.test
> > >
> > >   Expected Passes: 1466
> > >   Unsupported Tests  : 64
> > >   Unexpected Failures: 1
> > >
> > >
> > > I'm a little confused if I look at it because it modifies heavily a
> > > test but there's no code change associated? Did you by any chance
> > > forget to add a file?
> > > Anyway, please take a look and let me know if there's anything I can
> > > help you with.
> > >
> > > Best,
> > >
> > > --
> > > Davide
> > >
> > > On Tue, Feb 26, 2019 at 7:37 AM Tatyana Krasnukha via lldb-commits
> > >  wrote:
> > > >
> > > > Author: tkrasnukha
> > > > Date: Tue Feb 26 07:38:30 2019
> > > > New Revision: 354890
> > > >
> > > > URL:
> > > > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewv
> > > > c_
> > > > llvm-2Dproject-3Frev-3D354890-26view-3Drev=DwIBaQ=DPL6_X_6JkXF
> > > > x7
> > > > AXWqB0tg=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw=g6rcurKPv
> > > > oK
> > > > 7uD68Vi8adZfyKmJS9bRAr63MKkywSXk=-rE0GqyxP9jq55OdKHCMEDhWIfqRpjW
> > > > Xj
> > > > acBkREdhm8=
> > > > Log:
> > > > Fix short options syntax in Minidump test
> > > >
> > > > Modified:
> > > > lldb/trunk/lit/Minidump/dump-all.test
> > > >
> > > > Modified: lldb/trunk/lit/Minidump/dump-all.test
> > > > URL:
> > > > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewv
> > > > c_
> > > > llvm-2Dproject_lldb_trunk_lit_Minidump_dump-2Dall.test-3Frev-3D354
> > > > 89
> > > > 0-26r1-3D354889-26r2-3D354890-26view-3Ddiff=DwIBaQ=DPL6_X_6JkX
> > > > Fx
> > > > 7AXWqB0tg=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw=g6rcurKP
> > > > vo
> > > > K7uD68Vi8adZfyKmJS9bRAr63MKkywSXk=fhMMfmhf9JBV-5F86-iQ8U2aGD73sY
> > > > kJ
> > > > yY5lZd79lrQ=
> > > > ==
> > > > ==
> > > > ==
> > > > --- lldb/trunk/lit/Minidump/dump-all.test (original)
> > > > +++ lldb/trunk/lit/Minidump/dump-all.test Tue Feb 26 07:38:30 2019
> > > > @@ -11,32 +11,32 @@
> > > >  # RUN: --check-prefix=CHECKAUX --check-prefix=CHECKMAP \  # RUN:
> > > > --check-prefix=CHECKSTAT --check-prefix=CHECKUP
> > > > --check-prefix=CHECKFD %s  # RUN: %lldb -c
> > > > %p/Inputs/dump-content.dmp -o 'process plugin dump --directory' |
> > > > FileCheck --check-prefix=CHECKDIR %s -# RUN: %lldb -c
> > > > %p/Inputs/dump-content.dmp -o 'process plugin dump --d' |
> > > > FileCheck --check-prefix=CHECKDIR %s
> > > > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin
> > > > +dump -d' | FileCheck --check-prefix=CHECKDIR %s
> > > >  # RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin
> > > > dump --linux' | \  # RUN: FileCheck --check-prefix=CHECKCPU
> > > > --check-prefix=CHECKSTATUS \  # RUN: --check-prefix=CHECKLSB
> > > > --check-prefix=CHECKCMD --check-prefix=CHECKENV \  # RUN:
> > > > --check-prefix=CHECKAUX --check-prefix=CHECKMAP
> > > > --check-prefix=CHECKSTAT \  # RUN: --check-prefix=CHECKUP
> > > > --check-prefix=CHECKFD %s -# RUN: %lldb -c
> > > > %p/Inputs/dump-content.dmp -o 'process plugin dump --cpuinfo' |
> > > > FileCheck --check-prefix=CHECKCPU %s -# RUN: %lldb -c
> > > > %p/Inputs/dump-content.dmp -o 'process plugin dump --C' |
> > > > FileCheck --check-prefix=CHECKCPU %s -# RUN: %lldb -c
> > > > %p/Inputs/dump-content.dmp -o 'process plugin dump --status' |
> > 

Re: [Lldb-commits] [lldb] r354890 - Fix short options syntax in Minidump test

[Tatyana Krasnukha via lldb-commits]

This was fixed with r354890 ->  
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/20544/
Do you still have the issue?

-Original Message-
From: Davide Italiano  
Sent: Tuesday, February 26, 2019 7:28 PM
To: Tatyana Krasnukha 
Cc: lldb-commits 
Subject: Re: [Lldb-commits] [lldb] r354890 - Fix short options syntax in 
Minidump test

lldb: unrecognized option `--C'
error: unknown or ambiguous option
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/lldb/lit/Minidump/dump-all.test:55:13:
error: CHECKCPU: expected string not found in input

this seems to be the issue. Can you give another shot at fixing this or 
reverting? Thanks!

On Tue, Feb 26, 2019 at 8:16 AM Tatyana Krasnukha 
 wrote:
>
> Those build didn't contain r354890 yet.
>
> -Original Message-
> From: Davide Italiano 
> Sent: Tuesday, February 26, 2019 7:05 PM
> To: Tatyana Krasnukha 
> Cc: lldb-commits 
> Subject: Re: [Lldb-commits] [lldb] r354890 - Fix short options syntax 
> in Minidump test
>
> Link to the failure:
> https://urldefense.proofpoint.com/v2/url?u=http-3A__green.lab.llvm.org
> _green_job_lldb-2Dcmake__20543_console=DwIBaQ=DPL6_X_6JkXFx7AXWqB0
> tg=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw=g6rcurKPvoK7uD68Vi8
> adZfyKmJS9bRAr63MKkywSXk=jdhvETIBQFdrr0fN0efcvAVZ3EOQdqaYgbWsDZDdEEA
> =
>
> On Tue, Feb 26, 2019 at 8:04 AM Davide Italiano  wrote:
> >
> > Tatyana, this commit broke one of the bots:
> >
> > Testing Time: 315.65s
> > 
> > Failing Tests (1):
> > LLDB :: Minidump/dump-all.test
> >
> >   Expected Passes: 1466
> >   Unsupported Tests  : 64
> >   Unexpected Failures: 1
> >
> >
> > I'm a little confused if I look at it because it modifies heavily a 
> > test but there's no code change associated? Did you by any chance 
> > forget to add a file?
> > Anyway, please take a look and let me know if there's anything I can 
> > help you with.
> >
> > Best,
> >
> > --
> > Davide
> >
> > On Tue, Feb 26, 2019 at 7:37 AM Tatyana Krasnukha via lldb-commits 
> >  wrote:
> > >
> > > Author: tkrasnukha
> > > Date: Tue Feb 26 07:38:30 2019
> > > New Revision: 354890
> > >
> > > URL:
> > > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewv
> > > c_
> > > llvm-2Dproject-3Frev-3D354890-26view-3Drev=DwIBaQ=DPL6_X_6JkXF
> > > x7 
> > > AXWqB0tg=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw=g6rcurKPv
> > > oK 
> > > 7uD68Vi8adZfyKmJS9bRAr63MKkywSXk=-rE0GqyxP9jq55OdKHCMEDhWIfqRpjW
> > > Xj
> > > acBkREdhm8=
> > > Log:
> > > Fix short options syntax in Minidump test
> > >
> > > Modified:
> > > lldb/trunk/lit/Minidump/dump-all.test
> > >
> > > Modified: lldb/trunk/lit/Minidump/dump-all.test
> > > URL:
> > > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewv
> > > c_
> > > llvm-2Dproject_lldb_trunk_lit_Minidump_dump-2Dall.test-3Frev-3D354
> > > 89 
> > > 0-26r1-3D354889-26r2-3D354890-26view-3Ddiff=DwIBaQ=DPL6_X_6JkX
> > > Fx 
> > > 7AXWqB0tg=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw=g6rcurKP
> > > vo 
> > > K7uD68Vi8adZfyKmJS9bRAr63MKkywSXk=fhMMfmhf9JBV-5F86-iQ8U2aGD73sY
> > > kJ
> > > yY5lZd79lrQ=
> > > ==
> > > ==
> > > ==
> > > --- lldb/trunk/lit/Minidump/dump-all.test (original)
> > > +++ lldb/trunk/lit/Minidump/dump-all.test Tue Feb 26 07:38:30 2019
> > > @@ -11,32 +11,32 @@
> > >  # RUN: --check-prefix=CHECKAUX --check-prefix=CHECKMAP \  # RUN:
> > > --check-prefix=CHECKSTAT --check-prefix=CHECKUP 
> > > --check-prefix=CHECKFD %s  # RUN: %lldb -c 
> > > %p/Inputs/dump-content.dmp -o 'process plugin dump --directory' | 
> > > FileCheck --check-prefix=CHECKDIR %s -# RUN: %lldb -c 
> > > %p/Inputs/dump-content.dmp -o 'process plugin dump --d' | 
> > > FileCheck --check-prefix=CHECKDIR %s
> > > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin 
> > > +dump -d' | FileCheck --check-prefix=CHECKDIR %s
> > >  # RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin 
> > > dump --linux' | \  # RUN: FileCheck --check-prefix=CHECKCPU 
> > > --check-prefix=CHECKSTATUS \  # RUN: --check-prefix=CHECKLSB 
> > > --check-prefix=CHECKCMD --check-prefix=CHECKENV \  # RUN:
> > > --check-prefix=CHECKAUX --check-prefix=CHECKMAP 
> > > --check-prefix=CHECKSTAT \  # RUN: --check-prefix=CHECKUP 
> > > --check-prefix=CHECKFD %s -# RUN: %lldb -c 
> > > %p/Inputs/dump-content.dmp -o 'process plugin dump --cpuinfo' | 
> > > FileCheck --check-prefix=CHECKCPU %s -# RUN: %lldb -c 
> > > %p/Inputs/dump-content.dmp -o 'process plugin dump --C' | 
> > > FileCheck --check-prefix=CHECKCPU %s -# RUN: %lldb -c 
> > > %p/Inputs/dump-content.dmp -o 'process plugin dump --status' | 
> > > FileCheck --check-prefix=CHECKSTATUS %s -# RUN: %lldb -c 
> > > %p/Inputs/dump-content.dmp -o 'process plugin dump --s' | 
> > > FileCheck --check-prefix=CHECKSTATUS %s
> > > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > > --cpuinfo' | FileCheck --check-prefix=CHECKCPU %s
> > > +# RUN: %lldb 

Re: [Lldb-commits] [lldb] r354890 - Fix short options syntax in Minidump test

[Davide Italiano via lldb-commits]

lldb: unrecognized option `--C'
error: unknown or ambiguous option
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/lldb/lit/Minidump/dump-all.test:55:13:
error: CHECKCPU: expected string not found in input

this seems to be the issue. Can you give another shot at fixing this
or reverting? Thanks!

On Tue, Feb 26, 2019 at 8:16 AM Tatyana Krasnukha
 wrote:
>
> Those build didn't contain r354890 yet.
>
> -Original Message-
> From: Davide Italiano 
> Sent: Tuesday, February 26, 2019 7:05 PM
> To: Tatyana Krasnukha 
> Cc: lldb-commits 
> Subject: Re: [Lldb-commits] [lldb] r354890 - Fix short options syntax in 
> Minidump test
>
> Link to the failure:
> https://urldefense.proofpoint.com/v2/url?u=http-3A__green.lab.llvm.org_green_job_lldb-2Dcmake__20543_console=DwIBaQ=DPL6_X_6JkXFx7AXWqB0tg=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw=g6rcurKPvoK7uD68Vi8adZfyKmJS9bRAr63MKkywSXk=jdhvETIBQFdrr0fN0efcvAVZ3EOQdqaYgbWsDZDdEEA=
>
> On Tue, Feb 26, 2019 at 8:04 AM Davide Italiano  wrote:
> >
> > Tatyana, this commit broke one of the bots:
> >
> > Testing Time: 315.65s
> > 
> > Failing Tests (1):
> > LLDB :: Minidump/dump-all.test
> >
> >   Expected Passes: 1466
> >   Unsupported Tests  : 64
> >   Unexpected Failures: 1
> >
> >
> > I'm a little confused if I look at it because it modifies heavily a
> > test but there's no code change associated? Did you by any chance
> > forget to add a file?
> > Anyway, please take a look and let me know if there's anything I can
> > help you with.
> >
> > Best,
> >
> > --
> > Davide
> >
> > On Tue, Feb 26, 2019 at 7:37 AM Tatyana Krasnukha via lldb-commits
> >  wrote:
> > >
> > > Author: tkrasnukha
> > > Date: Tue Feb 26 07:38:30 2019
> > > New Revision: 354890
> > >
> > > URL:
> > > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_
> > > llvm-2Dproject-3Frev-3D354890-26view-3Drev=DwIBaQ=DPL6_X_6JkXFx7
> > > AXWqB0tg=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw=g6rcurKPvoK
> > > 7uD68Vi8adZfyKmJS9bRAr63MKkywSXk=-rE0GqyxP9jq55OdKHCMEDhWIfqRpjWXj
> > > acBkREdhm8=
> > > Log:
> > > Fix short options syntax in Minidump test
> > >
> > > Modified:
> > > lldb/trunk/lit/Minidump/dump-all.test
> > >
> > > Modified: lldb/trunk/lit/Minidump/dump-all.test
> > > URL:
> > > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_
> > > llvm-2Dproject_lldb_trunk_lit_Minidump_dump-2Dall.test-3Frev-3D35489
> > > 0-26r1-3D354889-26r2-3D354890-26view-3Ddiff=DwIBaQ=DPL6_X_6JkXFx
> > > 7AXWqB0tg=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw=g6rcurKPvo
> > > K7uD68Vi8adZfyKmJS9bRAr63MKkywSXk=fhMMfmhf9JBV-5F86-iQ8U2aGD73sYkJ
> > > yY5lZd79lrQ=
> > > 
> > > ==
> > > --- lldb/trunk/lit/Minidump/dump-all.test (original)
> > > +++ lldb/trunk/lit/Minidump/dump-all.test Tue Feb 26 07:38:30 2019
> > > @@ -11,32 +11,32 @@
> > >  # RUN: --check-prefix=CHECKAUX --check-prefix=CHECKMAP \  # RUN:
> > > --check-prefix=CHECKSTAT --check-prefix=CHECKUP
> > > --check-prefix=CHECKFD %s  # RUN: %lldb -c
> > > %p/Inputs/dump-content.dmp -o 'process plugin dump --directory' |
> > > FileCheck --check-prefix=CHECKDIR %s -# RUN: %lldb -c
> > > %p/Inputs/dump-content.dmp -o 'process plugin dump --d' | FileCheck
> > > --check-prefix=CHECKDIR %s
> > > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump
> > > +-d' | FileCheck --check-prefix=CHECKDIR %s
> > >  # RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump
> > > --linux' | \  # RUN: FileCheck --check-prefix=CHECKCPU
> > > --check-prefix=CHECKSTATUS \  # RUN: --check-prefix=CHECKLSB
> > > --check-prefix=CHECKCMD --check-prefix=CHECKENV \  # RUN:
> > > --check-prefix=CHECKAUX --check-prefix=CHECKMAP
> > > --check-prefix=CHECKSTAT \  # RUN: --check-prefix=CHECKUP
> > > --check-prefix=CHECKFD %s -# RUN: %lldb -c
> > > %p/Inputs/dump-content.dmp -o 'process plugin dump --cpuinfo' |
> > > FileCheck --check-prefix=CHECKCPU %s -# RUN: %lldb -c
> > > %p/Inputs/dump-content.dmp -o 'process plugin dump --C' | FileCheck
> > > --check-prefix=CHECKCPU %s -# RUN: %lldb -c
> > > %p/Inputs/dump-content.dmp -o 'process plugin dump --status' |
> > > FileCheck --check-prefix=CHECKSTATUS %s -# RUN: %lldb -c
> > > %p/Inputs/dump-content.dmp -o 'process plugin dump --s' | FileCheck
> > > --check-prefix=CHECKSTATUS %s
> > > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > > --cpuinfo' | FileCheck --check-prefix=CHECKCPU %s
> > > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -C'   
> > >  | FileCheck --check-prefix=CHECKCPU %s
> > > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > > --status'  | FileCheck --check-prefix=CHECKSTATUS %s
> > > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -s'   
> > >  | FileCheck --check-prefix=CHECKSTATUS %s
> > >  # RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin 

Re: [Lldb-commits] [lldb] r354890 - Fix short options syntax in Minidump test

[Tatyana Krasnukha via lldb-commits]

Those build didn't contain r354890 yet.

-Original Message-
From: Davide Italiano  
Sent: Tuesday, February 26, 2019 7:05 PM
To: Tatyana Krasnukha 
Cc: lldb-commits 
Subject: Re: [Lldb-commits] [lldb] r354890 - Fix short options syntax in 
Minidump test

Link to the failure:
https://urldefense.proofpoint.com/v2/url?u=http-3A__green.lab.llvm.org_green_job_lldb-2Dcmake__20543_console=DwIBaQ=DPL6_X_6JkXFx7AXWqB0tg=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw=g6rcurKPvoK7uD68Vi8adZfyKmJS9bRAr63MKkywSXk=jdhvETIBQFdrr0fN0efcvAVZ3EOQdqaYgbWsDZDdEEA=

On Tue, Feb 26, 2019 at 8:04 AM Davide Italiano  wrote:
>
> Tatyana, this commit broke one of the bots:
>
> Testing Time: 315.65s
> 
> Failing Tests (1):
> LLDB :: Minidump/dump-all.test
>
>   Expected Passes: 1466
>   Unsupported Tests  : 64
>   Unexpected Failures: 1
>
>
> I'm a little confused if I look at it because it modifies heavily a 
> test but there's no code change associated? Did you by any chance 
> forget to add a file?
> Anyway, please take a look and let me know if there's anything I can 
> help you with.
>
> Best,
>
> --
> Davide
>
> On Tue, Feb 26, 2019 at 7:37 AM Tatyana Krasnukha via lldb-commits 
>  wrote:
> >
> > Author: tkrasnukha
> > Date: Tue Feb 26 07:38:30 2019
> > New Revision: 354890
> >
> > URL: 
> > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_
> > llvm-2Dproject-3Frev-3D354890-26view-3Drev=DwIBaQ=DPL6_X_6JkXFx7
> > AXWqB0tg=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw=g6rcurKPvoK
> > 7uD68Vi8adZfyKmJS9bRAr63MKkywSXk=-rE0GqyxP9jq55OdKHCMEDhWIfqRpjWXj
> > acBkREdhm8=
> > Log:
> > Fix short options syntax in Minidump test
> >
> > Modified:
> > lldb/trunk/lit/Minidump/dump-all.test
> >
> > Modified: lldb/trunk/lit/Minidump/dump-all.test
> > URL: 
> > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_
> > llvm-2Dproject_lldb_trunk_lit_Minidump_dump-2Dall.test-3Frev-3D35489
> > 0-26r1-3D354889-26r2-3D354890-26view-3Ddiff=DwIBaQ=DPL6_X_6JkXFx
> > 7AXWqB0tg=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw=g6rcurKPvo
> > K7uD68Vi8adZfyKmJS9bRAr63MKkywSXk=fhMMfmhf9JBV-5F86-iQ8U2aGD73sYkJ
> > yY5lZd79lrQ= 
> > 
> > ==
> > --- lldb/trunk/lit/Minidump/dump-all.test (original)
> > +++ lldb/trunk/lit/Minidump/dump-all.test Tue Feb 26 07:38:30 2019
> > @@ -11,32 +11,32 @@
> >  # RUN: --check-prefix=CHECKAUX --check-prefix=CHECKMAP \  # RUN: 
> > --check-prefix=CHECKSTAT --check-prefix=CHECKUP 
> > --check-prefix=CHECKFD %s  # RUN: %lldb -c 
> > %p/Inputs/dump-content.dmp -o 'process plugin dump --directory' | 
> > FileCheck --check-prefix=CHECKDIR %s -# RUN: %lldb -c 
> > %p/Inputs/dump-content.dmp -o 'process plugin dump --d' | FileCheck 
> > --check-prefix=CHECKDIR %s
> > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > +-d' | FileCheck --check-prefix=CHECKDIR %s
> >  # RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > --linux' | \  # RUN: FileCheck --check-prefix=CHECKCPU 
> > --check-prefix=CHECKSTATUS \  # RUN: --check-prefix=CHECKLSB 
> > --check-prefix=CHECKCMD --check-prefix=CHECKENV \  # RUN: 
> > --check-prefix=CHECKAUX --check-prefix=CHECKMAP 
> > --check-prefix=CHECKSTAT \  # RUN: --check-prefix=CHECKUP 
> > --check-prefix=CHECKFD %s -# RUN: %lldb -c 
> > %p/Inputs/dump-content.dmp -o 'process plugin dump --cpuinfo' | 
> > FileCheck --check-prefix=CHECKCPU %s -# RUN: %lldb -c 
> > %p/Inputs/dump-content.dmp -o 'process plugin dump --C' | FileCheck 
> > --check-prefix=CHECKCPU %s -# RUN: %lldb -c 
> > %p/Inputs/dump-content.dmp -o 'process plugin dump --status' | 
> > FileCheck --check-prefix=CHECKSTATUS %s -# RUN: %lldb -c 
> > %p/Inputs/dump-content.dmp -o 'process plugin dump --s' | FileCheck 
> > --check-prefix=CHECKSTATUS %s
> > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > --cpuinfo' | FileCheck --check-prefix=CHECKCPU %s
> > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -C' 
> >| FileCheck --check-prefix=CHECKCPU %s
> > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > --status'  | FileCheck --check-prefix=CHECKSTATUS %s
> > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -s' 
> >| FileCheck --check-prefix=CHECKSTATUS %s
> >  # RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > --lsb-release' | FileCheck --check-prefix=CHECKLSB %s -# RUN: %lldb 
> > -c %p/Inputs/dump-content.dmp -o 'process plugin dump --r' | 
> > FileCheck --check-prefix=CHECKLSB %s -# RUN: %lldb -c 
> > %p/Inputs/dump-content.dmp -o 'process plugin dump --cmdline' | 
> > FileCheck --check-prefix=CHECKCMD %s -# RUN: %lldb -c 
> > %p/Inputs/dump-content.dmp -o 'process plugin dump --c' | FileCheck 
> > --check-prefix=CHECKCMD %s -# RUN: %lldb -c 
> > %p/Inputs/dump-content.dmp -o 'process plugin dump --environ' | 
> > FileCheck 

Re: [Lldb-commits] [lldb] r354890 - Fix short options syntax in Minidump test

[Davide Italiano via lldb-commits]

Link to the failure:
http://green.lab.llvm.org/green/job/lldb-cmake//20543/console

On Tue, Feb 26, 2019 at 8:04 AM Davide Italiano  wrote:
>
> Tatyana, this commit broke one of the bots:
>
> Testing Time: 315.65s
> 
> Failing Tests (1):
> LLDB :: Minidump/dump-all.test
>
>   Expected Passes: 1466
>   Unsupported Tests  : 64
>   Unexpected Failures: 1
>
>
> I'm a little confused if I look at it because it modifies heavily a
> test but there's no code change associated? Did you by any chance
> forget to add a file?
> Anyway, please take a look and let me know if there's anything I can
> help you with.
>
> Best,
>
> --
> Davide
>
> On Tue, Feb 26, 2019 at 7:37 AM Tatyana Krasnukha via lldb-commits
>  wrote:
> >
> > Author: tkrasnukha
> > Date: Tue Feb 26 07:38:30 2019
> > New Revision: 354890
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=354890=rev
> > Log:
> > Fix short options syntax in Minidump test
> >
> > Modified:
> > lldb/trunk/lit/Minidump/dump-all.test
> >
> > Modified: lldb/trunk/lit/Minidump/dump-all.test
> > URL: 
> > http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/dump-all.test?rev=354890=354889=354890=diff
> > ==
> > --- lldb/trunk/lit/Minidump/dump-all.test (original)
> > +++ lldb/trunk/lit/Minidump/dump-all.test Tue Feb 26 07:38:30 2019
> > @@ -11,32 +11,32 @@
> >  # RUN: --check-prefix=CHECKAUX --check-prefix=CHECKMAP \
> >  # RUN: --check-prefix=CHECKSTAT --check-prefix=CHECKUP 
> > --check-prefix=CHECKFD %s
> >  # RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > --directory' | FileCheck --check-prefix=CHECKDIR %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --d' | 
> > FileCheck --check-prefix=CHECKDIR %s
> > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -d' | 
> > FileCheck --check-prefix=CHECKDIR %s
> >  # RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > --linux' | \
> >  # RUN: FileCheck --check-prefix=CHECKCPU --check-prefix=CHECKSTATUS \
> >  # RUN: --check-prefix=CHECKLSB --check-prefix=CHECKCMD 
> > --check-prefix=CHECKENV \
> >  # RUN: --check-prefix=CHECKAUX --check-prefix=CHECKMAP 
> > --check-prefix=CHECKSTAT \
> >  # RUN: --check-prefix=CHECKUP --check-prefix=CHECKFD %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > --cpuinfo' | FileCheck --check-prefix=CHECKCPU %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --C' | 
> > FileCheck --check-prefix=CHECKCPU %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > --status' | FileCheck --check-prefix=CHECKSTATUS %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --s' | 
> > FileCheck --check-prefix=CHECKSTATUS %s
> > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > --cpuinfo' | FileCheck --check-prefix=CHECKCPU %s
> > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -C' 
> >| FileCheck --check-prefix=CHECKCPU %s
> > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > --status'  | FileCheck --check-prefix=CHECKSTATUS %s
> > +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -s' 
> >| FileCheck --check-prefix=CHECKSTATUS %s
> >  # RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > --lsb-release' | FileCheck --check-prefix=CHECKLSB %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --r' | 
> > FileCheck --check-prefix=CHECKLSB %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > --cmdline' | FileCheck --check-prefix=CHECKCMD %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --c' | 
> > FileCheck --check-prefix=CHECKCMD %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > --environ' | FileCheck --check-prefix=CHECKENV %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --e' | 
> > FileCheck --check-prefix=CHECKENV %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --auxv' 
> > | FileCheck --check-prefix=CHECKAUX %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --x' | 
> > FileCheck --check-prefix=CHECKAUX %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --maps' 
> > | FileCheck --check-prefix=CHECKMAP %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --m' | 
> > FileCheck --check-prefix=CHECKMAP %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --stat' 
> > | FileCheck --check-prefix=CHECKSTAT %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --S' | 
> > FileCheck --check-prefix=CHECKSTAT %s
> > -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> > --uptime' | FileCheck --check-prefix=CHECKUP %s
> > 

Re: [Lldb-commits] [lldb] r354890 - Fix short options syntax in Minidump test

[Davide Italiano via lldb-commits]

Tatyana, this commit broke one of the bots:

Testing Time: 315.65s

Failing Tests (1):
LLDB :: Minidump/dump-all.test

  Expected Passes: 1466
  Unsupported Tests  : 64
  Unexpected Failures: 1


I'm a little confused if I look at it because it modifies heavily a
test but there's no code change associated? Did you by any chance
forget to add a file?
Anyway, please take a look and let me know if there's anything I can
help you with.

Best,

--
Davide

On Tue, Feb 26, 2019 at 7:37 AM Tatyana Krasnukha via lldb-commits
 wrote:
>
> Author: tkrasnukha
> Date: Tue Feb 26 07:38:30 2019
> New Revision: 354890
>
> URL: http://llvm.org/viewvc/llvm-project?rev=354890=rev
> Log:
> Fix short options syntax in Minidump test
>
> Modified:
> lldb/trunk/lit/Minidump/dump-all.test
>
> Modified: lldb/trunk/lit/Minidump/dump-all.test
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/dump-all.test?rev=354890=354889=354890=diff
> ==
> --- lldb/trunk/lit/Minidump/dump-all.test (original)
> +++ lldb/trunk/lit/Minidump/dump-all.test Tue Feb 26 07:38:30 2019
> @@ -11,32 +11,32 @@
>  # RUN: --check-prefix=CHECKAUX --check-prefix=CHECKMAP \
>  # RUN: --check-prefix=CHECKSTAT --check-prefix=CHECKUP 
> --check-prefix=CHECKFD %s
>  # RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> --directory' | FileCheck --check-prefix=CHECKDIR %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --d' | 
> FileCheck --check-prefix=CHECKDIR %s
> +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -d' | 
> FileCheck --check-prefix=CHECKDIR %s
>  # RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --linux' 
> | \
>  # RUN: FileCheck --check-prefix=CHECKCPU --check-prefix=CHECKSTATUS \
>  # RUN: --check-prefix=CHECKLSB --check-prefix=CHECKCMD 
> --check-prefix=CHECKENV \
>  # RUN: --check-prefix=CHECKAUX --check-prefix=CHECKMAP 
> --check-prefix=CHECKSTAT \
>  # RUN: --check-prefix=CHECKUP --check-prefix=CHECKFD %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> --cpuinfo' | FileCheck --check-prefix=CHECKCPU %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --C' | 
> FileCheck --check-prefix=CHECKCPU %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --status' 
> | FileCheck --check-prefix=CHECKSTATUS %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --s' | 
> FileCheck --check-prefix=CHECKSTATUS %s
> +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> --cpuinfo' | FileCheck --check-prefix=CHECKCPU %s
> +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -C'   
>  | FileCheck --check-prefix=CHECKCPU %s
> +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --status' 
>  | FileCheck --check-prefix=CHECKSTATUS %s
> +# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -s'   
>  | FileCheck --check-prefix=CHECKSTATUS %s
>  # RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> --lsb-release' | FileCheck --check-prefix=CHECKLSB %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --r' | 
> FileCheck --check-prefix=CHECKLSB %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> --cmdline' | FileCheck --check-prefix=CHECKCMD %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --c' | 
> FileCheck --check-prefix=CHECKCMD %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
> --environ' | FileCheck --check-prefix=CHECKENV %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --e' | 
> FileCheck --check-prefix=CHECKENV %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --auxv' | 
> FileCheck --check-prefix=CHECKAUX %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --x' | 
> FileCheck --check-prefix=CHECKAUX %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --maps' | 
> FileCheck --check-prefix=CHECKMAP %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --m' | 
> FileCheck --check-prefix=CHECKMAP %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --stat' | 
> FileCheck --check-prefix=CHECKSTAT %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --S' | 
> FileCheck --check-prefix=CHECKSTAT %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --uptime' 
> | FileCheck --check-prefix=CHECKUP %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --u' | 
> FileCheck --check-prefix=CHECKUP %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --fd' | 
> FileCheck --check-prefix=CHECKFD %s
> -# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --f' | 
> FileCheck 

[Lldb-commits] [PATCH] D58630: [lldb] [test] Pass appropriate -L for just-built libc++

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

mgorny added a comment.

In D58630#1410419 , @labath wrote:

> Unfortunately I don't know of a substantially better way to solve. I have one 
> small improvement inline. Also, I take it this means running tests without a 
> libc++ checkout is not supported on NetBSD. Maybe you should add netbsd to 
> the cmake code where we print the darwin warning if you build without libc++ 
> checked out.


Actually, it can use either libc++ checkout or installed libc++. it just 
happens that the latter is uncommon.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58630



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

[Lldb-commits] [lldb] r354890 - Fix short options syntax in Minidump test

[Tatyana Krasnukha via lldb-commits]

Author: tkrasnukha
Date: Tue Feb 26 07:38:30 2019
New Revision: 354890

URL: http://llvm.org/viewvc/llvm-project?rev=354890=rev
Log:
Fix short options syntax in Minidump test

Modified:
lldb/trunk/lit/Minidump/dump-all.test

Modified: lldb/trunk/lit/Minidump/dump-all.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/dump-all.test?rev=354890=354889=354890=diff
==
--- lldb/trunk/lit/Minidump/dump-all.test (original)
+++ lldb/trunk/lit/Minidump/dump-all.test Tue Feb 26 07:38:30 2019
@@ -11,32 +11,32 @@
 # RUN: --check-prefix=CHECKAUX --check-prefix=CHECKMAP \
 # RUN: --check-prefix=CHECKSTAT --check-prefix=CHECKUP --check-prefix=CHECKFD 
%s
 # RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
--directory' | FileCheck --check-prefix=CHECKDIR %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --d' | 
FileCheck --check-prefix=CHECKDIR %s
+# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -d' | 
FileCheck --check-prefix=CHECKDIR %s
 # RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --linux' | \
 # RUN: FileCheck --check-prefix=CHECKCPU --check-prefix=CHECKSTATUS \
 # RUN: --check-prefix=CHECKLSB --check-prefix=CHECKCMD --check-prefix=CHECKENV 
\
 # RUN: --check-prefix=CHECKAUX --check-prefix=CHECKMAP 
--check-prefix=CHECKSTAT \
 # RUN: --check-prefix=CHECKUP --check-prefix=CHECKFD %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --cpuinfo' 
| FileCheck --check-prefix=CHECKCPU %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --C' | 
FileCheck --check-prefix=CHECKCPU %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --status' | 
FileCheck --check-prefix=CHECKSTATUS %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --s' | 
FileCheck --check-prefix=CHECKSTATUS %s
+# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --cpuinfo'  
   | FileCheck --check-prefix=CHECKCPU %s
+# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -C' 
   | FileCheck --check-prefix=CHECKCPU %s
+# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --status'   
   | FileCheck --check-prefix=CHECKSTATUS %s
+# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -s' 
   | FileCheck --check-prefix=CHECKSTATUS %s
 # RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump 
--lsb-release' | FileCheck --check-prefix=CHECKLSB %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --r' | 
FileCheck --check-prefix=CHECKLSB %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --cmdline' 
| FileCheck --check-prefix=CHECKCMD %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --c' | 
FileCheck --check-prefix=CHECKCMD %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --environ' 
| FileCheck --check-prefix=CHECKENV %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --e' | 
FileCheck --check-prefix=CHECKENV %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --auxv' | 
FileCheck --check-prefix=CHECKAUX %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --x' | 
FileCheck --check-prefix=CHECKAUX %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --maps' | 
FileCheck --check-prefix=CHECKMAP %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --m' | 
FileCheck --check-prefix=CHECKMAP %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --stat' | 
FileCheck --check-prefix=CHECKSTAT %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --S' | 
FileCheck --check-prefix=CHECKSTAT %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --uptime' | 
FileCheck --check-prefix=CHECKUP %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --u' | 
FileCheck --check-prefix=CHECKUP %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --fd' | 
FileCheck --check-prefix=CHECKFD %s
-# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --f' | 
FileCheck --check-prefix=CHECKFD %s
+# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -r' 
   | FileCheck --check-prefix=CHECKLSB %s
+# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --cmdline'  
   | FileCheck --check-prefix=CHECKCMD %s
+# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -c' 
   | FileCheck --check-prefix=CHECKCMD %s
+# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --environ'  
   | FileCheck --check-prefix=CHECKENV %s
+# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump -e' 
   | FileCheck --check-prefix=CHECKENV %s
+# RUN: %lldb -c %p/Inputs/dump-content.dmp -o 'process plugin dump --auxv' 
   | FileCheck 

[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

[Greg Clayton via Phabricator via lldb-commits]

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

I seem to remember that we currently expect that there are two cases when it 
comes to unknowns:

- specified unknowns
- unspecific unknowns

A "specified unknown" is when the user actually typed "unknown" in their 
triple. IIRC there is no "none" to specify that there is no OS, so we use 
"specified unknowns" for this case. In this case we expect the enum _and_ the 
string accessor value for vendor and/or OS to be the string "unknown".

"unspecified unkowns" is when the user just types "arrmv7" as the triple. We 
need to know they didn't specify anything for vendor and OS. In this case the 
enums return unknown, but the string accessor return values are empty.

So we can't really change this code unless we add a "none" enum and string 
value to the LLVM triple system so we can tell the difference. Right now we 
rely on the above mechanism in a lot of places around the code.


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

https://reviews.llvm.org/D58653



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

[Lldb-commits] [lldb] r354883 - Fix error handling in Options::Parse

[Tatyana Krasnukha via lldb-commits]

Author: tkrasnukha
Date: Tue Feb 26 06:50:40 2019
New Revision: 354883

URL: http://llvm.org/viewvc/llvm-project?rev=354883=rev
Log:
Fix error handling in Options::Parse

Moved `if (error.Fail())` to correct place to catch all faulty cases such as
"unknown or ambiguous option" which was ignored before.

Modified:
lldb/trunk/source/Interpreter/Options.cpp

Modified: lldb/trunk/source/Interpreter/Options.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Options.cpp?rev=354883=354882=354883=diff
==
--- lldb/trunk/source/Interpreter/Options.cpp (original)
+++ lldb/trunk/source/Interpreter/Options.cpp Tue Feb 26 06:50:40 2019
@@ -1436,10 +1436,11 @@ llvm::Expected Options::Parse(cons
 } else {
   error.SetErrorStringWithFormat("invalid option with value '%i'", val);
 }
-if (error.Fail())
-  return error.ToError();
   }
 
+  if (error.Fail())
+return error.ToError();
+
   argv.erase(argv.begin(), argv.begin() + OptionParser::GetOptionIndex());
   return ReconstituteArgsAfterParsing(argv, args);
 }


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

[Lldb-commits] [PATCH] D58350: Insert random blocks of python code with swig instead of modify-python-lldb.py

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Ping. :)


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

https://reviews.llvm.org/D58350



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

[Lldb-commits] [PATCH] D58610: [lldb] [lit] Set LD_LIBRARY_PATH or alike for Suite tests

[Pavel Labath via Phabricator via lldb-commits]

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

Doesn't seem particularly elegant, but it seems clang is doing the same thing, 
so probably best to follow suit.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58610



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

[Lldb-commits] [PATCH] D58630: [lldb] [test] Pass appropriate -L for just-built libc++

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Unfortunately I don't know of a substantially better way to solve. I have one 
small improvement inline. Also, I take it this means running tests without a 
libc++ checkout is not supported on NetBSD. Maybe you should add netbsd to the 
cmake code where we print the darwin warning if you build without libc++ 
checked out.




Comment at: lldb/lit/Suite/lit.cfg:35-36
 
+# Library path may be needed to locate just-built clang.
+config.environment['LLVM_LIBS_DIR'] = config.llvm_libs_dir
+

Instead of passing this via the environment, you should arrange so that `--env 
LLVM_LIBS_DIR=dir` gets passed to dotest.py. This will stop people from 
pounding the head against the wall on why do the tests behave differently when 
run from lit vs. directly from command line.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58630



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

[Lldb-commits] [PATCH] D58566: [Reproducers] Add more logging capabilities to reproducer instrumentation

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The overall idea seems fine to me, though I think it would be better to keep 
the macros simpler by moving all of the logic into the single `Register` call. 
I.e. the Register function would take extra StringRef arguments, which the 
macros would fill out. Then you could store the computed signature in the 
(possibly renamed) `m_ids` map, instead of having two maps with the same set of 
keys. That should save some space too.




Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:591
 
-LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "Recording ({0}) 
'{1}'",
+#ifndef LLDB_REPRO_INSTR_TRACE
+LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "Recording {0}: {1}",

aprantl wrote:
> Who defines this macro? I'm asking because it's unusual for LLDB to emit 
> anything on stderr.
I'm guessing it's the kind of thing you define by editing the source/cmake 
config, when you're really desperate to figure out what's going wrong.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58566



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

[Lldb-commits] [PATCH] D58564: [Reproducers] Add command provider

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I am sorry that I won't have much time to review this in the next couple of 
weeks, but I don't think this is a good direction here. I don't see how this 
will interact with the SB API recorder, specifically with things like 
SBCommandInterpreter::HandleCommand, and ::HandleCommandsFromFile. The thing I 
would expect to see is that SB recorder captures the input of those commands 
(for a somewhat broad interpretation of "capture") during recording, and then 
substitute this during replay. That way the CommandInterpreter class would not 
need (almost?) any modifications.

With this approach (shoving all commands into a single stream in the 
CommandInterpreter) it becomes impossible to replay the API calls above. If you 
want to proceed with this, then go ahead (it's your feature), but I believe 
you'll run into some problems down the line.


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

https://reviews.llvm.org/D58564



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

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I also think it would be great to group all host-specific functionality into a 
common library (other than Utility, since that already has a lot of other stuff 
in it).

I understand the desire to start with a clean slate for a new tool, so a new 
would-be-host folder might be a good middle ground between using host as-is and 
shoving everything into Utility. It will create some "git blame" churn, but 
probably won't be that bad overall. I probably wouldn't do it that way, because 
I think we're not far from "fixing" Host (you can help by reviewing D58167 
, I am waiting for your responses on the 
comments there), and I don't think it would be so bad to "accidentally" depend 
on everything in the interim. However, this seems fine too.

So, bottom line, if everyone is ok with the "staging" System folder, then I am 
too.


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

https://reviews.llvm.org/D58654



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