[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

2020-05-22 Thread Jaroslav Sevcik via Phabricator via lldb-commits
jarin added a comment.

In D80254#2047982 , @clayborg wrote:

> This looks good, thanks for subscribing me. We need to have GetNumChildren 
> and GetChildAtIndex agreeing on things and we definitely shouldn't be walking 
> more than on pointer recursively. My only question is do we need helper 
> functions added to TypeSystemClang to avoid this issue since we have 
> GetNumChildren and GetChildAtIndex doing things differently? Some function 
> both could/should be calling so that things can't get out of sync?


Yeah, that is what I suggested in the patch's summary. Do you want to block 
landing the fix until the refactoring is done? In my experience, the 
discussions about such refactorings can take weeks, I am not sure if I can 
commit to doing that. I am happy to put out a separate patch for the 
refactoring once this one is landed.


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

https://reviews.llvm.org/D80254



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


[Lldb-commits] [PATCH] D80408: [lldb/Test] Remove issue_verification subdirectory

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

Yes, these were intended to be unit tests for the test framework itself. A 
great idea, but it never really took off. I have never been able to get the 
initial tests that Todd added to work for me, and most of them are largely 
irrelevant by now.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D80408



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


[Lldb-commits] [lldb] bca378f - [lldb][NFC] Overload raw_ostream operator << for ConstString

2020-05-22 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-05-22T11:24:48+02:00
New Revision: bca378f68a7d21f9da7d2f86a54fdbb5604b4d05

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

LOG: [lldb][NFC] Overload raw_ostream operator << for ConstString

Summary: We are not doing this very often, but sometimes it's convenient when I 
can just << ConstStrings into llvm::errs() during testing.

Reviewers: labath, JDevlieghere

Reviewed By: labath, JDevlieghere

Subscribers: JDevlieghere

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

Added: 


Modified: 
lldb/include/lldb/Utility/ConstString.h
lldb/source/Core/Section.cpp
lldb/source/Symbol/Function.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/ConstString.h 
b/lldb/include/lldb/Utility/ConstString.h
index c2419407f2f6..1e55b2ebb957 100644
--- a/lldb/include/lldb/Utility/ConstString.h
+++ b/lldb/include/lldb/Utility/ConstString.h
@@ -490,6 +490,11 @@ template <> struct ScalarTraits 
{
   static QuotingType mustQuote(StringRef S) { return QuotingType::Double; }
 };
 } // namespace yaml
+
+inline raw_ostream &operator<<(raw_ostream &os, lldb_private::ConstString s) {
+  os << s.GetStringRef();
+  return os;
+}
 } // namespace llvm
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(lldb_private::ConstString)

diff  --git a/lldb/source/Core/Section.cpp b/lldb/source/Core/Section.cpp
index ce4715721ee7..9bf1c62c5ab8 100644
--- a/lldb/source/Core/Section.cpp
+++ b/lldb/source/Core/Section.cpp
@@ -337,7 +337,7 @@ void Section::DumpName(llvm::raw_ostream &s) const {
 if (name && name[0])
   s << name << '.';
   }
-  s << m_name.GetStringRef();
+  s << m_name;
 }
 
 bool Section::IsDescendant(const Section *section) {

diff  --git a/lldb/source/Symbol/Function.cpp b/lldb/source/Symbol/Function.cpp
index 0b1f6a8c3a3d..953c77632e01 100644
--- a/lldb/source/Symbol/Function.cpp
+++ b/lldb/source/Symbol/Function.cpp
@@ -374,9 +374,9 @@ void Function::GetDescription(Stream *s, 
lldb::DescriptionLevel level,
 
   *s << "id = " << (const UserID &)*this;
   if (name)
-*s << ", name = \"" << name.GetCString() << '"';
+s->AsRawOstream() << ", name = \"" << name << '"';
   if (mangled)
-*s << ", mangled = \"" << mangled.GetCString() << '"';
+s->AsRawOstream() << ", mangled = \"" << mangled << '"';
   *s << ", range = ";
   Address::DumpStyle fallback_style;
   if (level == eDescriptionLevelVerbose)



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


[Lldb-commits] [PATCH] D80310: [lldb][NFC] Overload raw_ostream operator << for ConstString

2020-05-22 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbca378f68a7d: [lldb][NFC] Overload raw_ostream operator 
<< for ConstString (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D80310?vs=265291&id=265685#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80310

Files:
  lldb/include/lldb/Utility/ConstString.h
  lldb/source/Core/Section.cpp
  lldb/source/Symbol/Function.cpp


Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -374,9 +374,9 @@
 
   *s << "id = " << (const UserID &)*this;
   if (name)
-*s << ", name = \"" << name.GetCString() << '"';
+s->AsRawOstream() << ", name = \"" << name << '"';
   if (mangled)
-*s << ", mangled = \"" << mangled.GetCString() << '"';
+s->AsRawOstream() << ", mangled = \"" << mangled << '"';
   *s << ", range = ";
   Address::DumpStyle fallback_style;
   if (level == eDescriptionLevelVerbose)
Index: lldb/source/Core/Section.cpp
===
--- lldb/source/Core/Section.cpp
+++ lldb/source/Core/Section.cpp
@@ -337,7 +337,7 @@
 if (name && name[0])
   s << name << '.';
   }
-  s << m_name.GetStringRef();
+  s << m_name;
 }
 
 bool Section::IsDescendant(const Section *section) {
Index: lldb/include/lldb/Utility/ConstString.h
===
--- lldb/include/lldb/Utility/ConstString.h
+++ lldb/include/lldb/Utility/ConstString.h
@@ -490,6 +490,11 @@
   static QuotingType mustQuote(StringRef S) { return QuotingType::Double; }
 };
 } // namespace yaml
+
+inline raw_ostream &operator<<(raw_ostream &os, lldb_private::ConstString s) {
+  os << s.GetStringRef();
+  return os;
+}
 } // namespace llvm
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(lldb_private::ConstString)


Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -374,9 +374,9 @@
 
   *s << "id = " << (const UserID &)*this;
   if (name)
-*s << ", name = \"" << name.GetCString() << '"';
+s->AsRawOstream() << ", name = \"" << name << '"';
   if (mangled)
-*s << ", mangled = \"" << mangled.GetCString() << '"';
+s->AsRawOstream() << ", mangled = \"" << mangled << '"';
   *s << ", range = ";
   Address::DumpStyle fallback_style;
   if (level == eDescriptionLevelVerbose)
Index: lldb/source/Core/Section.cpp
===
--- lldb/source/Core/Section.cpp
+++ lldb/source/Core/Section.cpp
@@ -337,7 +337,7 @@
 if (name && name[0])
   s << name << '.';
   }
-  s << m_name.GetStringRef();
+  s << m_name;
 }
 
 bool Section::IsDescendant(const Section *section) {
Index: lldb/include/lldb/Utility/ConstString.h
===
--- lldb/include/lldb/Utility/ConstString.h
+++ lldb/include/lldb/Utility/ConstString.h
@@ -490,6 +490,11 @@
   static QuotingType mustQuote(StringRef S) { return QuotingType::Double; }
 };
 } // namespace yaml
+
+inline raw_ostream &operator<<(raw_ostream &os, lldb_private::ConstString s) {
+  os << s.GetStringRef();
+  return os;
+}
 } // namespace llvm
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(lldb_private::ConstString)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 5f88f39 - [lldb] Enable C++14 when evaluating expressions in a C++14 frame

2020-05-22 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-05-22T11:42:44+02:00
New Revision: 5f88f39ab8154682c3b1eb9d7050a9412a55d9e7

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

LOG: [lldb] Enable C++14 when evaluating expressions in a C++14 frame

Summary:
Currently we never enable C++14 in the expression evaluator. This enables it 
when the language of the program is C++14.

It seems C++17 and so on isn't yet in any of the language enums (and the DWARF 
standard it seems), so C++17 support will be a follow up patch.

Reviewers: labath, JDevlieghere

Reviewed By: labath, JDevlieghere

Subscribers: aprantl

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

Added: 
lldb/test/API/lang/cpp/standards/cpp11/Makefile
lldb/test/API/lang/cpp/standards/cpp11/TestCPP11Standard.py
lldb/test/API/lang/cpp/standards/cpp11/main.cpp
lldb/test/API/lang/cpp/standards/cpp14/Makefile
lldb/test/API/lang/cpp/standards/cpp14/TestCPP14Standard.py
lldb/test/API/lang/cpp/standards/cpp14/main.cpp

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

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 8885cbc85b2c..877d75cfec30 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -491,9 +491,11 @@ ClangExpressionParser::ClangExpressionParser(
 // be re-evaluated in the future.
 lang_opts.CPlusPlus11 = true;
 break;
+  case lldb::eLanguageTypeC_plus_plus_14:
+lang_opts.CPlusPlus14 = true;
+LLVM_FALLTHROUGH;
   case lldb::eLanguageTypeC_plus_plus:
   case lldb::eLanguageTypeC_plus_plus_11:
-  case lldb::eLanguageTypeC_plus_plus_14:
 lang_opts.CPlusPlus11 = true;
 m_compiler->getHeaderSearchOpts().UseLibcxx = true;
 LLVM_FALLTHROUGH;

diff  --git a/lldb/test/API/lang/cpp/standards/cpp11/Makefile 
b/lldb/test/API/lang/cpp/standards/cpp11/Makefile
new file mode 100644
index ..e78030cbf752
--- /dev/null
+++ b/lldb/test/API/lang/cpp/standards/cpp11/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -std=c++11
+
+include Makefile.rules

diff  --git a/lldb/test/API/lang/cpp/standards/cpp11/TestCPP11Standard.py 
b/lldb/test/API/lang/cpp/standards/cpp11/TestCPP11Standard.py
new file mode 100644
index ..e4162c09758f
--- /dev/null
+++ b/lldb/test/API/lang/cpp/standards/cpp11/TestCPP11Standard.py
@@ -0,0 +1,19 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"))
+
+# Run an expression that is valid in C++11 (as it uses nullptr).
+self.expect_expr("nullptr == nullptr", result_type="bool", 
result_value="true")
+
+# Run a expression that is not valid in C++11 (as it uses polymorphic
+# lambdas from C++14).
+self.expect("expr [](auto x) { return x; }(1)", error=True, 
substrs=["'auto' not allowed in lambda parameter"])

diff  --git a/lldb/test/API/lang/cpp/standards/cpp11/main.cpp 
b/lldb/test/API/lang/cpp/standards/cpp11/main.cpp
new file mode 100644
index ..ba45ee316cd4
--- /dev/null
+++ b/lldb/test/API/lang/cpp/standards/cpp11/main.cpp
@@ -0,0 +1,3 @@
+int main() {
+  return 0; // break here
+}

diff  --git a/lldb/test/API/lang/cpp/standards/cpp14/Makefile 
b/lldb/test/API/lang/cpp/standards/cpp14/Makefile
new file mode 100644
index ..a27336ffd9ac
--- /dev/null
+++ b/lldb/test/API/lang/cpp/standards/cpp14/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -std=c++14
+
+include Makefile.rules

diff  --git a/lldb/test/API/lang/cpp/standards/cpp14/TestCPP14Standard.py 
b/lldb/test/API/lang/cpp/standards/cpp14/TestCPP14Standard.py
new file mode 100644
index ..3422cf19b3d7
--- /dev/null
+++ b/lldb/test/API/lang/cpp/standards/cpp14/TestCPP14Standard.py
@@ -0,0 +1,19 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"))
+
+# Run an expression that is valid in C++11 (as it uses nullptr).
+self.expect_expr("nullptr == nullptr", result_type="bool", 
result_value="true")
+
+# Run a expression 

[Lldb-commits] [PATCH] D80308: [lldb] Enable C++14 when evaluating expressions in a C++14 frame

2020-05-22 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5f88f39ab815: [lldb] Enable C++14 when evaluating 
expressions in a C++14 frame (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80308

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/test/API/lang/cpp/standards/cpp11/Makefile
  lldb/test/API/lang/cpp/standards/cpp11/TestCPP11Standard.py
  lldb/test/API/lang/cpp/standards/cpp11/main.cpp
  lldb/test/API/lang/cpp/standards/cpp14/Makefile
  lldb/test/API/lang/cpp/standards/cpp14/TestCPP14Standard.py
  lldb/test/API/lang/cpp/standards/cpp14/main.cpp


Index: lldb/test/API/lang/cpp/standards/cpp14/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/standards/cpp14/main.cpp
@@ -0,0 +1,3 @@
+int main() {
+  return 0; // break here
+}
Index: lldb/test/API/lang/cpp/standards/cpp14/TestCPP14Standard.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/standards/cpp14/TestCPP14Standard.py
@@ -0,0 +1,19 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"))
+
+# Run an expression that is valid in C++11 (as it uses nullptr).
+self.expect_expr("nullptr == nullptr", result_type="bool", 
result_value="true")
+
+# Run a expression that is only valid in C++14 that (as it uses
+# polymorphic lambdas).
+self.expect_expr("[](auto x) { return x; }(1)", result_type="int", 
result_value="1")
Index: lldb/test/API/lang/cpp/standards/cpp14/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/standards/cpp14/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -std=c++14
+
+include Makefile.rules
Index: lldb/test/API/lang/cpp/standards/cpp11/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/standards/cpp11/main.cpp
@@ -0,0 +1,3 @@
+int main() {
+  return 0; // break here
+}
Index: lldb/test/API/lang/cpp/standards/cpp11/TestCPP11Standard.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/standards/cpp11/TestCPP11Standard.py
@@ -0,0 +1,19 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"))
+
+# Run an expression that is valid in C++11 (as it uses nullptr).
+self.expect_expr("nullptr == nullptr", result_type="bool", 
result_value="true")
+
+# Run a expression that is not valid in C++11 (as it uses polymorphic
+# lambdas from C++14).
+self.expect("expr [](auto x) { return x; }(1)", error=True, 
substrs=["'auto' not allowed in lambda parameter"])
Index: lldb/test/API/lang/cpp/standards/cpp11/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/standards/cpp11/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -std=c++11
+
+include Makefile.rules
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -491,9 +491,11 @@
 // be re-evaluated in the future.
 lang_opts.CPlusPlus11 = true;
 break;
+  case lldb::eLanguageTypeC_plus_plus_14:
+lang_opts.CPlusPlus14 = true;
+LLVM_FALLTHROUGH;
   case lldb::eLanguageTypeC_plus_plus:
   case lldb::eLanguageTypeC_plus_plus_11:
-  case lldb::eLanguageTypeC_plus_plus_14:
 lang_opts.CPlusPlus11 = true;
 m_compiler->getHeaderSearchOpts().UseLibcxx = true;
 LLVM_FALLTHROUGH;


Index: lldb/test/API/lang/cpp/standards/cpp14/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/standards/cpp14/main.cpp
@@ -0,0 +1,3 @@
+int main() {
+  return 0; // break here
+}
Index: lldb/test/API/lang/cpp/standards/cpp14/TestCPP14Standard.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/standards/cpp14/TestCPP

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-05-22 Thread Ilya Bukonkin via Phabricator via lldb-commits
fallkrum marked an inline comment as done.
fallkrum added inline comments.



Comment at: lldb/source/Target/Thread.cpp:382
+if (m_stop_info_sp->IsValid() ||
+(IsStillAtLastBreakpointHit() &&
+ m_resume_state != eStateSuspended) ||

What is historical need for this check? How is it possible for a breakpoint to 
stop a thread that was already stopped second time even while stepping in 
multithreaded programs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80112



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


[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

2020-05-22 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor accepted this revision.
teemperor added a comment.

I think the refactoring is a good idea, but IMHO this patch can go in as-is. 
It's not adding any more debt to the existing approach (it even deletes code) 
and it adds tests, so I don't see a drawback of merging this. Also having this 
bug fixed with a small back portable patch seems better than doing it with some 
big refactor.


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

https://reviews.llvm.org/D80254



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


[Lldb-commits] [lldb] 053b063 - [lldb] Increase timeout in TestExitDuringExpression

2020-05-22 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-05-22T12:47:34+02:00
New Revision: 053b0634ea93b1ed7993adf34730bd752e9d84ec

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

LOG: [lldb] Increase timeout in TestExitDuringExpression

200 microseconds is not enough time for any expression to execute
reliably. On linux, calling pthread_exit can result in call to dlopen,
which cannot complete in that time, particularly when running under a
debugger.

On linux, this test failed all the time, on macos, about two thirds of
runs were failing.  This patch increases the timeout to 100ms, which is
enough to get it passing reliably on linux, though I wouldn't be
surprised if an even bigger timeout would be needed for remote test
runs.

Added: 


Modified: 
lldb/test/API/functionalities/thread/exit_during_expression/main.c

Removed: 




diff  --git 
a/lldb/test/API/functionalities/thread/exit_during_expression/main.c 
b/lldb/test/API/functionalities/thread/exit_during_expression/main.c
index 66b6018976cf..a3bf53915a03 100644
--- a/lldb/test/API/functionalities/thread/exit_during_expression/main.c
+++ b/lldb/test/API/functionalities/thread/exit_during_expression/main.c
@@ -3,7 +3,7 @@
 #include 
 #include 
 
-static unsigned int g_timeout = 200;
+static unsigned int g_timeout = 10;
 
 int function_to_call() {
 



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


[Lldb-commits] [lldb] d89c98a - [lldb/Test] Remove issue_verification subdirectory

2020-05-22 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-05-22T09:32:12-07:00
New Revision: d89c98a020c9903f31b659e3f5cc513ea878c900

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

LOG: [lldb/Test] Remove issue_verification subdirectory

These files haven't been touched since 2015. According to Pavel these
were intended to be test for the test framework which never really took
of and are mostly irrelevant by now.

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

Added: 


Modified: 


Removed: 
lldb/test/API/issue_verification/README.txt
lldb/test/API/issue_verification/TestExpectedTimeout.py.park
lldb/test/API/issue_verification/TestFail.py.park
lldb/test/API/issue_verification/TestInvalidDecorator.py.park
lldb/test/API/issue_verification/TestRerunFail.py.park
lldb/test/API/issue_verification/TestRerunFileLevelTimeout.py.park
lldb/test/API/issue_verification/TestRerunInline.py.park
lldb/test/API/issue_verification/TestRerunTimeout.py.park
lldb/test/API/issue_verification/TestSignal.py.park
lldb/test/API/issue_verification/TestSignalOutsideTestMethod.py.park
lldb/test/API/issue_verification/TestTimeout.py.park
lldb/test/API/issue_verification/disable.py
lldb/test/API/issue_verification/enable.py
lldb/test/API/issue_verification/inline_rerun_inferior.cpp
lldb/test/API/issue_verification/rerun_base.py



diff  --git a/lldb/test/API/issue_verification/README.txt 
b/lldb/test/API/issue_verification/README.txt
deleted file mode 100644
index 0f1ae7f0ecfc..
--- a/lldb/test/API/issue_verification/README.txt
+++ /dev/null
@@ -1,5 +0,0 @@
-Tests in this directory are intentionally setup to
-fail, error, timeout, etc. to verify that the buildbots
-pick up errors.  The tests in this directory will be
-parked/removed/renamed after verifying they trigger
-as expected.

diff  --git a/lldb/test/API/issue_verification/TestExpectedTimeout.py.park 
b/lldb/test/API/issue_verification/TestExpectedTimeout.py.park
deleted file mode 100644
index 67db8149f850..
--- a/lldb/test/API/issue_verification/TestExpectedTimeout.py.park
+++ /dev/null
@@ -1,20 +0,0 @@
-"""Tests that a timeout is detected by the testbot."""
-from __future__ import print_function
-
-import time
-
-import lldbsuite.test.lldbtest as lldbtest
-
-
-class ExpectedTimeoutTestCase(lldbtest.TestBase):
-"""Forces test timeout."""
-mydir = lldbtest.TestBase.compute_mydir(__file__)
-
-@lldbtest.expectedFailureAll()
-def test_buildbot_sees_expected_timeout(self):
-"""Tests that expected timeout logic kicks in and is picked up."""
-while True:
-try:
-time.sleep(1)
-except:
-print("ignoring exception during sleep")

diff  --git a/lldb/test/API/issue_verification/TestFail.py.park 
b/lldb/test/API/issue_verification/TestFail.py.park
deleted file mode 100644
index fcdba39d7fe7..
--- a/lldb/test/API/issue_verification/TestFail.py.park
+++ /dev/null
@@ -1,15 +0,0 @@
-"""Tests that a FAIL is detected by the testbot."""
-
-
-import lldbsuite.test.lldbtest as lldbtest
-
-
-class FailTestCase(lldbtest.TestBase):
-"""Forces test failure."""
-mydir = lldbtest.TestBase.compute_mydir(__file__)
-
-def test_buildbot_catches_failure(self):
-"""Issues a failing test assertion."""
-self.assertTrue(
-False,
-"This will always fail, buildbot should flag this.")

diff  --git a/lldb/test/API/issue_verification/TestInvalidDecorator.py.park 
b/lldb/test/API/issue_verification/TestInvalidDecorator.py.park
deleted file mode 100644
index d5c8c58506fd..
--- a/lldb/test/API/issue_verification/TestInvalidDecorator.py.park
+++ /dev/null
@@ -1,12 +0,0 @@
-from lldbsuite.test import lldbtest
-from lldbsuite.test import decorators
-
-
-class NonExistentDecoratorTestCase(lldbtest.TestBase):
-
-mydir = lldbtest.TestBase.compute_mydir(__file__)
-
-@decorators.nonExistentDecorator(bugnumber="yt/1300")
-def test(self):
-"""Verify non-existent decorators are picked up by test runner."""
-pass

diff  --git a/lldb/test/API/issue_verification/TestRerunFail.py.park 
b/lldb/test/API/issue_verification/TestRerunFail.py.park
deleted file mode 100644
index 8d1fc95502ad..
--- a/lldb/test/API/issue_verification/TestRerunFail.py.park
+++ /dev/null
@@ -1,22 +0,0 @@
-"""Tests that a flakey fail is rerun, and will pass on the rerun.
-Run this test with --rerun-all-issues specified to test that
-the tests fail on the first run, then pass on the second.
-Do not mark them as flakey as, at this time, flakey tests will
-run twice, thus causing the second run to succeed."""
-
-
-import rerun_base
-
-import lldbsuite.test.lldbtest as lld

[Lldb-commits] [PATCH] D80408: [lldb/Test] Remove issue_verification subdirectory

2020-05-22 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd89c98a020c9: [lldb/Test] Remove issue_verification 
subdirectory (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80408

Files:
  lldb/test/API/issue_verification/README.txt
  lldb/test/API/issue_verification/TestExpectedTimeout.py.park
  lldb/test/API/issue_verification/TestFail.py.park
  lldb/test/API/issue_verification/TestInvalidDecorator.py.park
  lldb/test/API/issue_verification/TestRerunFail.py.park
  lldb/test/API/issue_verification/TestRerunFileLevelTimeout.py.park
  lldb/test/API/issue_verification/TestRerunInline.py.park
  lldb/test/API/issue_verification/TestRerunTimeout.py.park
  lldb/test/API/issue_verification/TestSignal.py.park
  lldb/test/API/issue_verification/TestSignalOutsideTestMethod.py.park
  lldb/test/API/issue_verification/TestTimeout.py.park
  lldb/test/API/issue_verification/disable.py
  lldb/test/API/issue_verification/enable.py
  lldb/test/API/issue_verification/inline_rerun_inferior.cpp
  lldb/test/API/issue_verification/rerun_base.py

Index: lldb/test/API/issue_verification/rerun_base.py
===
--- lldb/test/API/issue_verification/rerun_base.py
+++ /dev/null
@@ -1,27 +0,0 @@
-
-import os
-
-import lldbsuite.test.lldbtest as lldbtest
-
-
-# pylint: disable=too-few-public-methods
-class RerunBaseTestCase(lldbtest.TestBase):
-"""Forces test failure."""
-mydir = lldbtest.TestBase.compute_mydir(__file__)
-
-def should_generate_issue(self):
-"""Returns whether a test issue should be generated.
-
-@returns True on the first and every other call via a given
-test method.
-"""
-should_pass_filename = "{}.{}.succeed-marker".format(
-__file__, self.id())
-fail = not os.path.exists(should_pass_filename)
-if fail:
-# Create the marker so that next call to this passes.
-open(should_pass_filename, 'w').close()
-else:
-# Delete the marker so next time we fail.
-os.remove(should_pass_filename)
-return fail
Index: lldb/test/API/issue_verification/inline_rerun_inferior.cpp
===
--- lldb/test/API/issue_verification/inline_rerun_inferior.cpp
+++ /dev/null
@@ -1,6 +0,0 @@
-typedef int Foo;
-
-int main() {
-Foo array[3] = {1,2,3};
-return 0; //% self.expect("frame variable array --show-types --", substrs = ['(Foo [3]) wrong_type_here = {','(Foo) [0] = 1','(Foo) [1] = 2','(Foo) [2] = 3'])
-}
Index: lldb/test/API/issue_verification/enable.py
===
--- lldb/test/API/issue_verification/enable.py
+++ /dev/null
@@ -1,20 +0,0 @@
-#!/usr/bin/env python
-"""Renames *.py.park files to *.py."""
-import os
-import sys
-
-
-def main():
-"""Drives the main script behavior."""
-script_dir = os.path.dirname(os.path.realpath(__file__))
-for filename in os.listdir(script_dir):
-basename, extension = os.path.splitext(filename)
-if basename.startswith("Test") and extension == '.park':
-source_path = os.path.join(script_dir, filename)
-dest_path = os.path.join(script_dir, basename)
-sys.stdout.write("renaming {} to {}\n".format(
-source_path, dest_path))
-os.rename(source_path, dest_path)
-
-if __name__ == "__main__":
-main()
Index: lldb/test/API/issue_verification/disable.py
===
--- lldb/test/API/issue_verification/disable.py
+++ /dev/null
@@ -1,20 +0,0 @@
-#!/usr/bin/env python
-"""Renames *.py files to *.py.park."""
-import os
-import sys
-
-
-def main():
-"""Drives the main script behavior."""
-script_dir = os.path.dirname(os.path.realpath(__file__))
-for filename in os.listdir(script_dir):
-basename, extension = os.path.splitext(filename)
-if basename.startswith("Test") and extension == '.py':
-source_path = os.path.join(script_dir, filename)
-dest_path = source_path + ".park"
-sys.stdout.write("renaming {} to {}\n".format(
-source_path, dest_path))
-os.rename(source_path, dest_path)
-
-if __name__ == "__main__":
-main()
Index: lldb/test/API/issue_verification/TestTimeout.py.park
===
--- lldb/test/API/issue_verification/TestTimeout.py.park
+++ /dev/null
@@ -1,19 +0,0 @@
-"""Tests that a timeout is detected by the testbot."""
-from __future__ import print_function
-
-import time
-
-import lldbsuite.test.lldbtest as lldbtest
-
-
-class TimeoutTestCase(lldbtest.TestBase):
-"""Forces test timeout."""
-mydir = lldbtest.TestBase.compute

Re: [Lldb-commits] [lldb] 8a6333e - [lldb/REPL] Fix unhandled switch case

2020-05-22 Thread Jim Ingham via lldb-commits
Sorry I missed that.  That seems fine.  In the dedicated REPL, you’re probably 
dead at this point, but you can do “expr —repl --” in which case we’ll pick 
whatever thread on to run your expression, adding the results to the REPL 
context, so it could happen there and you might not be all the way toast.  So 
just returning an error is fine.

Jim

> On May 21, 2020, at 11:23 PM, Jonas Devlieghere  wrote:
> 
> Jim, does this look correct to you?
> 
> On Thu, May 21, 2020 at 11:22 PM Jonas Devlieghere via lldb-commits 
> mailto:lldb-commits@lists.llvm.org>> wrote:
> 
> Author: Jonas Devlieghere
> Date: 2020-05-21T23:22:17-07:00
> New Revision: 8a6333ef38088b65224bc021a14f0a123a29
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/8a6333ef38088b65224bc021a14f0a123a29
>  
> 
> DIFF: 
> https://github.com/llvm/llvm-project/commit/8a6333ef38088b65224bc021a14f0a123a29.diff
>  
> 
> 
> LOG: [lldb/REPL] Fix unhandled switch case
> 
> Fix warning: enumeration value 'eExpressionThreadVanished' not handled
> in switch [-Wswitch]
> 
> Added: 
> 
> 
> Modified: 
> lldb/source/Expression/REPL.cpp
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/lldb/source/Expression/REPL.cpp 
> b/lldb/source/Expression/REPL.cpp
> index 6c9792c6e837..a55fe09bdeb6 100644
> --- a/lldb/source/Expression/REPL.cpp
> +++ b/lldb/source/Expression/REPL.cpp
> @@ -388,6 +388,11 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, 
> std::string &code) {
>  error_sp->Printf("error: stopped for debug -- %s\n",
>   error.AsCString());
>  break;
> +  case lldb::eExpressionThreadVanished:
> +// Shoulnd't happen???
> +error_sp->Printf("error: expression thread vanished -- %s\n",
> + error.AsCString());
> +break;
>}
>  }
> 
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org 
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits 
> 

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


Re: [Lldb-commits] [lldb] 053b063 - [lldb] Increase timeout in TestExitDuringExpression

2020-05-22 Thread Jim Ingham via lldb-commits
I set the timeouts low just to keep the test from taking too long.  Beyond that 
there’s no harm in bumping them up till they pass consistently.

I messed with the timeouts and reran the test a bunch of times till it was 
stable for me, but I have a pretty fast mac, so apparently that wasn’t a good 
test…

I don’t see a way to do this without timeouts, however…

Thanks for fixing this.

Jim


> On May 22, 2020, at 3:47 AM, Pavel Labath via lldb-commits 
>  wrote:
> 
> 
> Author: Pavel Labath
> Date: 2020-05-22T12:47:34+02:00
> New Revision: 053b0634ea93b1ed7993adf34730bd752e9d84ec
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/053b0634ea93b1ed7993adf34730bd752e9d84ec
> DIFF: 
> https://github.com/llvm/llvm-project/commit/053b0634ea93b1ed7993adf34730bd752e9d84ec.diff
> 
> LOG: [lldb] Increase timeout in TestExitDuringExpression
> 
> 200 microseconds is not enough time for any expression to execute
> reliably. On linux, calling pthread_exit can result in call to dlopen,
> which cannot complete in that time, particularly when running under a
> debugger.
> 
> On linux, this test failed all the time, on macos, about two thirds of
> runs were failing.  This patch increases the timeout to 100ms, which is
> enough to get it passing reliably on linux, though I wouldn't be
> surprised if an even bigger timeout would be needed for remote test
> runs.
> 
> Added: 
> 
> 
> Modified: 
>lldb/test/API/functionalities/thread/exit_during_expression/main.c
> 
> Removed: 
> 
> 
> 
> 
> diff  --git 
> a/lldb/test/API/functionalities/thread/exit_during_expression/main.c 
> b/lldb/test/API/functionalities/thread/exit_during_expression/main.c
> index 66b6018976cf..a3bf53915a03 100644
> --- a/lldb/test/API/functionalities/thread/exit_during_expression/main.c
> +++ b/lldb/test/API/functionalities/thread/exit_during_expression/main.c
> @@ -3,7 +3,7 @@
> #include 
> #include 
> 
> -static unsigned int g_timeout = 200;
> +static unsigned int g_timeout = 10;
> 
> int function_to_call() {
> 
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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


[Lldb-commits] [PATCH] D80448: [lldb/Test] Add a trace method to replace (commented out) print statements.

2020-05-22 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: labath, teemperor, aprantl.
Herald added a subscriber: abidh.

While debugging replay failures I noticed that a lot of tests have print 
statements, sometimes commented out, which half of the time don't show up in 
the terminal anyway because dotest swallows the output.

We already have a good mechanism for these kind of things: the `-t` flag to 
enable tracing. I've added a `trace` method to replace these print statements. 
In this patch I've only updated the ones that are commented out, but I feel all 
the print statements in the API tests should be replaced by this.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D80448

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  
lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
  lldb/test/API/lang/c/register_variables/TestRegisterVariables.py
  lldb/test/API/lang/objc/blocks/TestObjCIvarsInBlocks.py
  lldb/test/API/tools/lldb-server/TestGdbRemoteAuxvSupport.py
  lldb/test/API/tools/lldb-server/TestGdbRemoteExpeditedRegisters.py
  lldb/test/API/tools/lldb-server/TestGdbRemoteRegisterState.py
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -10,9 +10,6 @@
 the initial set of tests implemented.
 """
 
-from __future__ import division, print_function
-
-
 import unittest2
 import gdbremote_testcase
 import lldbgdbserverutils
@@ -1442,7 +1439,7 @@
 # Write flipped bit pattern of existing value to each register.
 (successful_writes, failed_writes) = self.flip_all_bits_in_each_register_value(
 gpr_reg_infos, endian)
-# print("successful writes: {}, failed writes: {}".format(successful_writes, failed_writes))
+self.trace("successful writes: {}, failed writes: {}".format(successful_writes, failed_writes))
 self.assertTrue(successful_writes > 0)
 
 # Note: as of this moment, a hefty number of the GPR writes are failing with E32 (everything except rax-rdx, rdi, rsi, rbp).
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteRegisterState.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteRegisterState.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteRegisterState.py
@@ -1,6 +1,3 @@
-from __future__ import print_function
-
-
 import gdbremote_testcase
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -50,7 +47,7 @@
 self.assertIsNotNone(threads)
 thread_id = threads[0]
 self.assertIsNotNone(thread_id)
-# print("Running on thread: 0x{:x}".format(thread_id))
+self.trace("Running on thread: 0x{:x}".format(thread_id))
 else:
 thread_id = None
 
@@ -64,22 +61,22 @@
 (success, state_id) = self.parse_QSaveRegisterState_response(context)
 self.assertTrue(success)
 self.assertIsNotNone(state_id)
-# print("saved register state id: {}".format(state_id))
+self.trace("saved register state id: {}".format(state_id))
 
 # Remember initial register values.
 initial_reg_values = self.read_register_values(
 gpr_reg_infos, endian, thread_id=thread_id)
-# print("initial_reg_values: {}".format(initial_reg_values))
+self.trace("initial_reg_values: {}".format(initial_reg_values))
 
 # Flip gpr register values.
 (successful_writes, failed_writes) = self.flip_all_bits_in_each_register_value(
 gpr_reg_infos, endian, thread_id=thread_id)
-# print("successful writes: {}, failed writes: {}".format(successful_writes, failed_writes))
+self.trace("successful writes: {}, failed writes: {}".format(successful_writes, failed_writes))
 self.assertTrue(successful_writes > 0)
 
 flipped_reg_values = self.read_register_values(
 gpr_reg_infos, endian, thread_id=thread_id)
-# print("flipped_reg_values: {}".format(flipped_reg_values))
+self.trace("flipped_reg_values: {}".format(flipped_reg_values))
 
 # Restore register values.
 self.reset_test_sequence()
@@ -91,7 +88,7 @@
 # Verify registers match initial register values.
 final_reg_values = self.read_register_values(
 gpr_reg_infos, endian, thread_id=thread_id)
-# print("final_reg_values: {}".format(final_reg_values))
+self.trace("final_reg_values: {}".format(final_reg_values))
 self.assertIsNotNone(final_reg_values)
 self.assertEqual(final_reg_values, initial_reg_values)
 
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteExpeditedRegisters.py
===
--- lldb/test/

[Lldb-commits] [PATCH] D80418: Print a warning when stopped in a frame LLDB has no plugin for.

2020-05-22 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/source/Target/Process.cpp:5808
+void Process::PrintWarningUnsupportedLanguage(const SymbolContext &sc) {
+  if (!GetWarningsUnsupportedLanguage())
+return;

For `eWarningsOptimization` this check happens in `PrintWarning`. I think your 
approach is better, but it'd be nice for the two to be consistent. 


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

https://reviews.llvm.org/D80418



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


[Lldb-commits] [PATCH] D80308: [lldb] Enable C++14 when evaluating expressions in a C++14 frame

2020-05-22 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments.



Comment at: lldb/test/API/lang/cpp/standards/cpp14/TestCPP14Standard.py:19
+# polymorphic lambdas).
+self.expect_expr("[](auto x) { return x; }(1)", result_type="int", 
result_value="1")

It would be worth it to add a more complete set of C++14 tests. 
[p1319r0](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1319r0.html) 
covers the C++11 to C++14 differences. 

Especially ones that may not work so we can file bugs for these and track them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80308



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


[Lldb-commits] [PATCH] D80448: [lldb/Test] Add a trace method to replace (commented out) print statements.

2020-05-22 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:510
+with recording(self, self.TraceOn()) as sbuf:
+print(args, kwargs, file=sbuf)
+

This should be 
```
print(*args, kwargs, file=sbuf)
```


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D80448



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


[Lldb-commits] [PATCH] D80308: [lldb] Enable C++14 when evaluating expressions in a C++14 frame

2020-05-22 Thread Max Kudryavtsev via Phabricator via lldb-commits
max-kudr added a comment.

Hello,

I suppose this commit broke Windows LLDB tests 
(http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/16486 -> logs: 
http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/16486/steps/test/logs/stdio).
 Can you please fix or rollback it?

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80308



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


Re: [Lldb-commits] [PATCH] D80308: [lldb] Enable C++14 when evaluating expressions in a C++14 frame

2020-05-22 Thread Raphael “Teemperor” Isemann via lldb-commits
Sorry, somehow I missed that mail. Reverted.

> On 22. May 2020, at 21:20, Max Kudryavtsev via Phabricator 
>  wrote:
> 
> max-kudr added a comment.
> 
> Hello,
> 
> I suppose this commit broke Windows LLDB tests 
> (http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/16486 -> 
> logs: 
> http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/16486/steps/test/logs/stdio).
>  Can you please fix or rollback it?
> 
> Thank you!
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D80308/new/
> 
> https://reviews.llvm.org/D80308
> 
> 
> 

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


[Lldb-commits] [lldb] 8cb7574 - Revert "[lldb] Enable C++14 when evaluating expressions in a C++14 frame"

2020-05-22 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-05-22T21:23:03+02:00
New Revision: 8cb75745412e4bc9592d2409cc6cfa4a2940d9e7

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

LOG: Revert "[lldb] Enable C++14 when evaluating expressions in a C++14 frame"

This reverts commit 5f88f39ab8154682c3b1eb9d7050a9412a55d9e7. It broke these
three tests on the Window bot:
  lldb-api :: commands/expression/completion/TestExprCompletion.py
  lldb-api :: lang/cpp/scope/TestCppScope.py
  lldb-api :: lang/cpp/standards/cpp11/TestCPP11Standard.py

Added: 


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

Removed: 
lldb/test/API/lang/cpp/standards/cpp11/Makefile
lldb/test/API/lang/cpp/standards/cpp11/TestCPP11Standard.py
lldb/test/API/lang/cpp/standards/cpp11/main.cpp
lldb/test/API/lang/cpp/standards/cpp14/Makefile
lldb/test/API/lang/cpp/standards/cpp14/TestCPP14Standard.py
lldb/test/API/lang/cpp/standards/cpp14/main.cpp



diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 877d75cfec30..8885cbc85b2c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -491,11 +491,9 @@ ClangExpressionParser::ClangExpressionParser(
 // be re-evaluated in the future.
 lang_opts.CPlusPlus11 = true;
 break;
-  case lldb::eLanguageTypeC_plus_plus_14:
-lang_opts.CPlusPlus14 = true;
-LLVM_FALLTHROUGH;
   case lldb::eLanguageTypeC_plus_plus:
   case lldb::eLanguageTypeC_plus_plus_11:
+  case lldb::eLanguageTypeC_plus_plus_14:
 lang_opts.CPlusPlus11 = true;
 m_compiler->getHeaderSearchOpts().UseLibcxx = true;
 LLVM_FALLTHROUGH;

diff  --git a/lldb/test/API/lang/cpp/standards/cpp11/Makefile 
b/lldb/test/API/lang/cpp/standards/cpp11/Makefile
deleted file mode 100644
index e78030cbf752..
--- a/lldb/test/API/lang/cpp/standards/cpp11/Makefile
+++ /dev/null
@@ -1,4 +0,0 @@
-CXX_SOURCES := main.cpp
-CXXFLAGS_EXTRAS := -std=c++11
-
-include Makefile.rules

diff  --git a/lldb/test/API/lang/cpp/standards/cpp11/TestCPP11Standard.py 
b/lldb/test/API/lang/cpp/standards/cpp11/TestCPP11Standard.py
deleted file mode 100644
index e4162c09758f..
--- a/lldb/test/API/lang/cpp/standards/cpp11/TestCPP11Standard.py
+++ /dev/null
@@ -1,19 +0,0 @@
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-class TestCase(TestBase):
-
-mydir = TestBase.compute_mydir(__file__)
-
-def test(self):
-self.build()
-lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"))
-
-# Run an expression that is valid in C++11 (as it uses nullptr).
-self.expect_expr("nullptr == nullptr", result_type="bool", 
result_value="true")
-
-# Run a expression that is not valid in C++11 (as it uses polymorphic
-# lambdas from C++14).
-self.expect("expr [](auto x) { return x; }(1)", error=True, 
substrs=["'auto' not allowed in lambda parameter"])

diff  --git a/lldb/test/API/lang/cpp/standards/cpp11/main.cpp 
b/lldb/test/API/lang/cpp/standards/cpp11/main.cpp
deleted file mode 100644
index ba45ee316cd4..
--- a/lldb/test/API/lang/cpp/standards/cpp11/main.cpp
+++ /dev/null
@@ -1,3 +0,0 @@
-int main() {
-  return 0; // break here
-}

diff  --git a/lldb/test/API/lang/cpp/standards/cpp14/Makefile 
b/lldb/test/API/lang/cpp/standards/cpp14/Makefile
deleted file mode 100644
index a27336ffd9ac..
--- a/lldb/test/API/lang/cpp/standards/cpp14/Makefile
+++ /dev/null
@@ -1,4 +0,0 @@
-CXX_SOURCES := main.cpp
-CXXFLAGS_EXTRAS := -std=c++14
-
-include Makefile.rules

diff  --git a/lldb/test/API/lang/cpp/standards/cpp14/TestCPP14Standard.py 
b/lldb/test/API/lang/cpp/standards/cpp14/TestCPP14Standard.py
deleted file mode 100644
index 3422cf19b3d7..
--- a/lldb/test/API/lang/cpp/standards/cpp14/TestCPP14Standard.py
+++ /dev/null
@@ -1,19 +0,0 @@
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-class TestCase(TestBase):
-
-mydir = TestBase.compute_mydir(__file__)
-
-def test(self):
-self.build()
-lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"))
-
-# Run an expression that is valid in C++11 (as it uses nullptr).
-self.expect_expr("nullptr == nullptr", result_type="bool", 
result_value="true")
-
-# Run a expression that is only valid in C++14 that (as it uses
-# polymorphic lambdas).
-self.expect_expr("[](auto

[Lldb-commits] [lldb] 5a85582 - [lldb/Reproducers] Make the type tests work with reproducers

2020-05-22 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-05-22T13:07:10-07:00
New Revision: 5a85582eb26f0c8f6b8ef5a14385d608ef10385c

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

LOG: [lldb/Reproducers] Make the type tests work with reproducers

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/test/API/types/AbstractBase.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 9d119e08b6c3..b02181ae1ffc 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -667,6 +667,13 @@ def getBuildDir(self):
 return os.path.join(os.environ["LLDB_BUILD"], self.mydir,
 self.getBuildDirBasename())
 
+def getReproducerDir(self):
+"""Return the full path to the reproducer if enabled."""
+if configuration.capture_path:
+return configuration.capture_path
+if configuration.replay_path:
+return configuration.replay_path
+return None
 
 def makeBuildDir(self):
 """Create the test-specific working directory, deleting any previous
@@ -685,6 +692,9 @@ def getSourcePath(self, name):
 """Return absolute path to a file in the test's source directory."""
 return os.path.join(self.getSourceDir(), name)
 
+def getReproducerArtifact(self, name):
+return os.path.join(self.getReproducerDir(), name)
+
 @classmethod
 def setUpCommands(cls):
 commands = [

diff  --git a/lldb/test/API/types/AbstractBase.py 
b/lldb/test/API/types/AbstractBase.py
index f2abfa092a7e..df8416b9c41f 100644
--- a/lldb/test/API/types/AbstractBase.py
+++ b/lldb/test/API/types/AbstractBase.py
@@ -33,12 +33,15 @@ def setUp(self):
 # module cacheing subsystem to be confused with executable name "a.out"
 # used for all the test cases.
 self.exe_name = self.testMethodName
-self.golden_filename = self.getBuildArtifact("golden-output.txt")
+golden = "{}-golden-output.txt".format(self.testMethodName)
+if configuration.is_reproducer():
+self.golden_filename = self.getReproducerArtifact(golden)
+else:
+self.golden_filename = self.getBuildArtifact(golden)
 
 def tearDown(self):
 """Cleanup the test byproducts."""
-#print("Removing golden-output.txt...")
-if os.path.exists(self.golden_filename):
+if os.path.exists(self.golden_filename) and not 
configuration.is_reproducer():
 os.remove(self.golden_filename)
 TestBase.tearDown(self)
 
@@ -88,7 +91,7 @@ def build_and_run_with_source_atoms_expr(
 blockCaptured=bc,
 quotedDisplay=qd)
 
-def process_launch_o(self, localPath):
+def process_launch_o(self):
 # process launch command output redirect always goes to host the
 # process is running on
 if lldb.remote_platform:
@@ -101,10 +104,32 @@ def process_launch_o(self, localPath):
 # copy remote_path to local host
 self.runCmd('platform get-file {remote} "{local}"'.format(
 remote=remote_path, local=self.golden_filename))
+elif configuration.is_reproducer_replay():
+# Don't overwrite the golden file generated at capture time.
+self.runCmd('process launch')
 else:
 self.runCmd(
 'process launch -o 
"{local}"'.format(local=self.golden_filename))
 
+def get_golden_list(self, blockCaptured=False):
+with open(self.golden_filename, 'r') as f:
+go = f.read()
+
+golden_list = []
+# Scan the golden output line by line, looking for the pattern:
+#
+# variable = 'value'
+#
+for line in go.split(os.linesep):
+# We'll ignore variables of array types from inside a block.
+if blockCaptured and '[' in line:
+continue
+match = self.pattern.search(line)
+if match:
+var, val = match.group(1), match.group(2)
+golden_list.append((var, val))
+return golden_list
+
 def generic_type_tester(
 self,
 exe_name,
@@ -117,29 +142,11 @@ def generic_type_tester(
 
 # First, capture the golden output emitted by the oracle, i.e., the
 # series of printf statements.
-
-self.process_launch_o(self.golden_filename)
-
-with open(self.golden_filename) as f:
-go = f.read()
+self.process_launch_o()
 
 # This golden list contains a list of (variable, value) pairs extracted
 # from the golden output.
-gl = []
-
-  

[Lldb-commits] [lldb] a67b2fa - [lldb/Test] Disable APITests.exe on Windows

2020-05-22 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-05-22T13:07:10-07:00
New Revision: a67b2faa7c4cfbceffb4213f46769c45a5a9291a

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

LOG: [lldb/Test] Disable APITests.exe on Windows

The generated binary (APITests.exe) is not a valid googletest binary. I
suspect it has something to do with us linking against liblldb.

Added: 


Modified: 
lldb/unittests/CMakeLists.txt

Removed: 




diff  --git a/lldb/unittests/CMakeLists.txt b/lldb/unittests/CMakeLists.txt
index eab053f03650..42aa2c2d7567 100644
--- a/lldb/unittests/CMakeLists.txt
+++ b/lldb/unittests/CMakeLists.txt
@@ -59,7 +59,10 @@ function(add_unittest_inputs test_name inputs)
 endfunction()
 
 add_subdirectory(TestingSupport)
-add_subdirectory(API)
+if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
+  # FIXME: APITests.exe is not a valid googletest binary.
+  add_subdirectory(API)
+endif()
 add_subdirectory(Breakpoint)
 add_subdirectory(Core)
 add_subdirectory(DataFormatter)



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


Re: [Lldb-commits] [lldb] a67b2fa - [lldb/Test] Disable APITests.exe on Windows

2020-05-22 Thread Jonas Devlieghere via lldb-commits
+ Walter

On Fri, May 22, 2020 at 1:07 PM Jonas Devlieghere via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

>
> Author: Jonas Devlieghere
> Date: 2020-05-22T13:07:10-07:00
> New Revision: a67b2faa7c4cfbceffb4213f46769c45a5a9291a
>
> URL:
> https://github.com/llvm/llvm-project/commit/a67b2faa7c4cfbceffb4213f46769c45a5a9291a
> DIFF:
> https://github.com/llvm/llvm-project/commit/a67b2faa7c4cfbceffb4213f46769c45a5a9291a.diff
>
> LOG: [lldb/Test] Disable APITests.exe on Windows
>
> The generated binary (APITests.exe) is not a valid googletest binary. I
> suspect it has something to do with us linking against liblldb.
>
> Added:
>
>
> Modified:
> lldb/unittests/CMakeLists.txt
>
> Removed:
>
>
>
>
> 
> diff  --git a/lldb/unittests/CMakeLists.txt b/lldb/unittests/CMakeLists.txt
> index eab053f03650..42aa2c2d7567 100644
> --- a/lldb/unittests/CMakeLists.txt
> +++ b/lldb/unittests/CMakeLists.txt
> @@ -59,7 +59,10 @@ function(add_unittest_inputs test_name inputs)
>  endfunction()
>
>  add_subdirectory(TestingSupport)
> -add_subdirectory(API)
> +if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
> +  # FIXME: APITests.exe is not a valid googletest binary.
> +  add_subdirectory(API)
> +endif()
>  add_subdirectory(Breakpoint)
>  add_subdirectory(Core)
>  add_subdirectory(DataFormatter)
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 7510aed - Handle eExpressionThreadVanished in error switch to handle

2020-05-22 Thread Eric Christopher via lldb-commits

Author: Eric Christopher
Date: 2020-05-22T13:43:10-07:00
New Revision: 7510aede627267819d9693381ad6c16dccfa0d17

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

LOG: Handle eExpressionThreadVanished in error switch to handle
covered switch warning.

Added: 


Modified: 
lldb/source/Interpreter/CommandInterpreter.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index df19855b5f8c..1cd71b07eaeb 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1620,6 +1620,11 @@ Status CommandInterpreter::PreprocessCommand(std::string 
&command) {
"expression '%s'",
expr_str.c_str());
 break;
+  case eExpressionThreadVanished:
+error.SetErrorStringWithFormat(
+"expression thread vanished for the expression '%s'",
+expr_str.c_str());
+break;
   }
 }
   }



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


[Lldb-commits] [PATCH] D80308: [lldb] Enable C++14 when evaluating expressions in a C++14 frame

2020-05-22 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

> It seems C++17 and so on isn't yet in any of the language enums (and the 
> DWARF standard it seems), so C++17 support will be a follow up patch.

Yes and that may point to a problem with this approach. IIRC, there is — on 
purpose — no C++17 language constant because the committee didn't see the need 
to differentiate it in DWARF, since all features were additive and could be 
described through specific C++17-related attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80308



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


[Lldb-commits] [PATCH] D80308: [lldb] Enable C++14 when evaluating expressions in a C++14 frame

2020-05-22 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added a comment.

In D80308#2051624 , @aprantl wrote:

> > It seems C++17 and so on isn't yet in any of the language enums (and the 
> > DWARF standard it seems), so C++17 support will be a follow up patch.
>
> Yes and that may point to a problem with this approach. IIRC, there is — on 
> purpose — no C++17 language constant because the committee didn't see the 
> need to differentiate it in DWARF, since all features were additive and could 
> be described through specific C++17-related attributes.


That is also problematic b/c you can still compile in C++17 mode but not use 
any C++17 specific constructs and yet expect LLDB to act as-if.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80308



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


[Lldb-commits] [PATCH] D80418: Print a warning when stopped in a frame LLDB has no plugin for.

2020-05-22 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl updated this revision to Diff 265808.
aprantl added a comment.

Address review feedback and add a similar test for the previously untested 
optimization warning.


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

https://reviews.llvm.org/D80418

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Target/Process.cpp
  lldb/source/Target/TargetProperties.td
  lldb/source/Target/Thread.cpp
  lldb/test/Shell/Process/Inputs/true.c
  lldb/test/Shell/Process/Optimization.test
  lldb/test/Shell/Process/UnsupportedLanguage.test

Index: lldb/test/Shell/Process/UnsupportedLanguage.test
===
--- /dev/null
+++ lldb/test/Shell/Process/UnsupportedLanguage.test
@@ -0,0 +1,8 @@
+Test warnings.
+REQUIRES: shell
+RUN: %clang_host %S/Inputs/true.c -std=c99 -g -c -S -emit-llvm -o - \
+RUN:   | sed -e 's/DW_LANG_C99/DW_LANG_PLI/g' >%t.ll
+RUN: %clang_host %t.ll -g -o %t.exe
+RUN: %lldb -o "b main" -o r -o q -b %t.exe | FileCheck %s
+
+CHECK: This version of LLDB has no plugin for the pli language
Index: lldb/test/Shell/Process/Optimization.test
===
--- /dev/null
+++ lldb/test/Shell/Process/Optimization.test
@@ -0,0 +1,6 @@
+Test warnings.
+REQUIRES: shell
+RUN: %clang_host -O3 %S/Inputs/true.c -std=c99 -g -o %t.exe
+RUN: %lldb -o "b main" -o r -o q -b %t.exe | FileCheck %s
+
+CHECK: compiled with optimization
Index: lldb/test/Shell/Process/Inputs/true.c
===
--- /dev/null
+++ lldb/test/Shell/Process/Inputs/true.c
@@ -0,0 +1,3 @@
+int main(int argc, char **argv) {
+  return 0;
+}
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -326,10 +326,13 @@
   if (!frame)
 return;
 
-  if (frame->HasDebugInformation() && GetProcess()->GetWarningsOptimization()) {
+  if (frame->HasDebugInformation() &&
+  (GetProcess()->GetWarningsOptimization() ||
+   GetProcess()->GetWarningsUnsupportedLanguage())) {
 SymbolContext sc =
 frame->GetSymbolContext(eSymbolContextFunction | eSymbolContextModule);
 GetProcess()->PrintWarningOptimization(sc);
+GetProcess()->PrintWarningUnsupportedLanguage(sc);
   }
 }
 
Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -204,6 +204,9 @@
   def WarningOptimization: Property<"optimization-warnings", "Boolean">,
 DefaultTrue,
 Desc<"If true, warn when stopped in code that is optimized where stepping and variable availability may not behave as expected.">;
+  def WarningUnsupportedLanguage: Property<"unsupported-language-warnings", "Boolean">,
+DefaultTrue,
+Desc<"If true, warn when stopped in code that is written in a source language that LLDB does not support.">;
   def StopOnExec: Property<"stop-on-exec", "Boolean">,
 Global,
 DefaultTrue,
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -258,6 +258,12 @@
   nullptr, idx, g_process_properties[idx].default_uint_value != 0);
 }
 
+bool ProcessProperties::GetWarningsUnsupportedLanguage() const {
+  const uint32_t idx = ePropertyWarningUnsupportedLanguage;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_process_properties[idx].default_uint_value != 0);
+}
+
 bool ProcessProperties::GetStopOnExec() const {
   const uint32_t idx = ePropertyStopOnExec;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
@@ -5761,9 +5767,6 @@
   StreamSP stream_sp = GetTarget().GetDebugger().GetAsyncOutputStream();
   if (!stream_sp)
 return;
-  if (warning_type == eWarningsOptimization && !GetWarningsOptimization()) {
-return;
-  }
 
   if (repeat_key != nullptr) {
 WarningsCollection::iterator it = m_warnings_issued.find(warning_type);
@@ -5788,8 +5791,11 @@
 }
 
 void Process::PrintWarningOptimization(const SymbolContext &sc) {
-  if (GetWarningsOptimization() && sc.module_sp &&
-  !sc.module_sp->GetFileSpec().GetFilename().IsEmpty() && sc.function &&
+  if (!GetWarningsOptimization())
+return;
+  if (!sc.module_sp)
+return;
+  if (!sc.module_sp->GetFileSpec().GetFilename().IsEmpty() && sc.function &&
   sc.function->GetIsOptimized()) {
 PrintWarning(Process::Warnings::eWarningsOptimization, sc.module_sp.get(),
  "%s was compiled with optimization - stepping may behave "
@@ -5798,6 +5804,25 @@
   }
 }
 
+void Process::PrintWarningUnsupportedLanguage(const SymbolContext &sc) {
+  if (!GetWarningsUnsupportedLanguage())
+return;
+  if (!sc.module_sp)
+return;
+  LanguageType language = sc.GetLanguage();
+  if (language ==

[Lldb-commits] [PATCH] D80418: Print a warning when stopped in a frame LLDB has no plugin for.

2020-05-22 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM!


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

https://reviews.llvm.org/D80418



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


[Lldb-commits] [PATCH] D80308: [lldb] Enable C++14 when evaluating expressions in a C++14 frame

2020-05-22 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor marked an inline comment as done.
teemperor added a comment.

In D80308#2051624 , @aprantl wrote:

> > It seems C++17 and so on isn't yet in any of the language enums (and the 
> > DWARF standard it seems), so C++17 support will be a follow up patch.
>
> Yes and that may point to a problem with this approach. IIRC, there is — on 
> purpose — no C++17 language constant because the committee didn't see the 
> need to differentiate it in DWARF, since all features were additive and could 
> be described through specific C++17-related attributes.


Interesting, so DWARF will do 11, 14, , 20? And 17 will presumably be 
encoded as 14 in DWARF?




Comment at: lldb/test/API/lang/cpp/standards/cpp14/TestCPP14Standard.py:19
+# polymorphic lambdas).
+self.expect_expr("[](auto x) { return x; }(1)", result_type="int", 
result_value="1")

shafik wrote:
> It would be worth it to add a more complete set of C++14 tests. 
> [p1319r0](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1319r0.html)
>  covers the C++11 to C++14 differences. 
> 
> Especially ones that may not work so we can file bugs for these and track 
> them.
We're only checking that the right Clang language standard was set, not Clang's 
C++ features related to that set standard (which should be and probably already 
is a test in Clang).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80308



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


[Lldb-commits] [lldb] 220c17f - Print a warning when stopped in a frame LLDB has no plugin for.

2020-05-22 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2020-05-22T15:37:36-07:00
New Revision: 220c17ffd4e1b127bcc02b25980b7934184ee1da

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

LOG: Print a warning when stopped in a frame LLDB has no plugin for.

This patchs adds an optional warning that is printed when stopped at a
frame that was compiled in a source language that LLDB has no plugin
for.

The motivational use-case is debugging Swift code on Linux. When the
user accidentally invokes the system LLDB that was built without the
Swift plugin, it is very much non-obvious why debugging doesnt
work. This warning makes it easy to figure out what went wrong.



Added: 
lldb/test/Shell/Process/Inputs/true.c
lldb/test/Shell/Process/Optimization.test
lldb/test/Shell/Process/UnsupportedLanguage.test

Modified: 
lldb/include/lldb/Target/Process.h
lldb/source/Target/Process.cpp
lldb/source/Target/TargetProperties.td
lldb/source/Target/Thread.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index fe811deb7caa..6f31741b4ce6 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -86,6 +86,7 @@ class ProcessProperties : public Properties {
   bool GetDetachKeepsStopped() const;
   void SetDetachKeepsStopped(bool keep_stopped);
   bool GetWarningsOptimization() const;
+  bool GetWarningsUnsupportedLanguage() const;
   bool GetStopOnExec() const;
   std::chrono::seconds GetUtilityExpressionTimeout() const;
   bool GetOSPluginReportsAllThreads() const;
@@ -390,7 +391,7 @@ class Process : public 
std::enable_shared_from_this,
   };
 
   /// Process warning types.
-  enum Warnings { eWarningsOptimization = 1 };
+  enum Warnings { eWarningsOptimization = 1, eWarningsUnsupportedLanguage = 2 
};
 
   typedef Range LoadRange;
   // We use a read/write lock to allow on or more clients to access the process
@@ -1319,6 +1320,12 @@ class Process : public 
std::enable_shared_from_this,
   /// pre-computed.
   void PrintWarningOptimization(const SymbolContext &sc);
 
+  /// Print a user-visible warning about a function written in a
+  /// language that this version of LLDB doesn't support.
+  ///
+  /// \see PrintWarningOptimization
+  void PrintWarningUnsupportedLanguage(const SymbolContext &sc);
+
   virtual bool GetProcessInfo(ProcessInstanceInfo &info);
 
 public:

diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 0eb9866e4a4b..25a940b66430 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -258,6 +258,12 @@ bool ProcessProperties::GetWarningsOptimization() const {
   nullptr, idx, g_process_properties[idx].default_uint_value != 0);
 }
 
+bool ProcessProperties::GetWarningsUnsupportedLanguage() const {
+  const uint32_t idx = ePropertyWarningUnsupportedLanguage;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_process_properties[idx].default_uint_value != 0);
+}
+
 bool ProcessProperties::GetStopOnExec() const {
   const uint32_t idx = ePropertyStopOnExec;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
@@ -5779,9 +5785,6 @@ void Process::PrintWarning(uint64_t warning_type, const 
void *repeat_key,
   StreamSP stream_sp = GetTarget().GetDebugger().GetAsyncOutputStream();
   if (!stream_sp)
 return;
-  if (warning_type == eWarningsOptimization && !GetWarningsOptimization()) {
-return;
-  }
 
   if (repeat_key != nullptr) {
 WarningsCollection::iterator it = m_warnings_issued.find(warning_type);
@@ -5806,8 +5809,11 @@ void Process::PrintWarning(uint64_t warning_type, const 
void *repeat_key,
 }
 
 void Process::PrintWarningOptimization(const SymbolContext &sc) {
-  if (GetWarningsOptimization() && sc.module_sp &&
-  !sc.module_sp->GetFileSpec().GetFilename().IsEmpty() && sc.function &&
+  if (!GetWarningsOptimization())
+return;
+  if (!sc.module_sp)
+return;
+  if (!sc.module_sp->GetFileSpec().GetFilename().IsEmpty() && sc.function &&
   sc.function->GetIsOptimized()) {
 PrintWarning(Process::Warnings::eWarningsOptimization, sc.module_sp.get(),
  "%s was compiled with optimization - stepping may behave "
@@ -5816,6 +5822,25 @@ void Process::PrintWarningOptimization(const 
SymbolContext &sc) {
   }
 }
 
+void Process::PrintWarningUnsupportedLanguage(const SymbolContext &sc) {
+  if (!GetWarningsUnsupportedLanguage())
+return;
+  if (!sc.module_sp)
+return;
+  LanguageType language = sc.GetLanguage();
+  if (language == eLanguageTypeUnknown)
+return;
+  auto type_system_or_err = sc.module_sp->GetTypeSystemForLanguage(language);
+  if (auto err = type_system_or_err.takeError()) {
+llvm::consumeError(std::move(err));
+PrintWarning(Process::

[Lldb-commits] [lldb] a8a048a - Restrict test for DW_AT_APPLE_optimized to Darwin

2020-05-22 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2020-05-22T15:52:00-07:00
New Revision: a8a048ac7253ca92228d0862b6c1175f1613a840

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

LOG: Restrict test for DW_AT_APPLE_optimized to Darwin

Added: 


Modified: 
lldb/test/Shell/Process/Optimization.test

Removed: 




diff  --git a/lldb/test/Shell/Process/Optimization.test 
b/lldb/test/Shell/Process/Optimization.test
index 10d241f698c0..1309a16aa98e 100644
--- a/lldb/test/Shell/Process/Optimization.test
+++ b/lldb/test/Shell/Process/Optimization.test
@@ -1,5 +1,5 @@
 Test warnings.
-REQUIRES: shell
+REQUIRES: shell, system-darwin
 RUN: %clang_host -O3 %S/Inputs/true.c -std=c99 -g -o %t.exe
 RUN: %lldb -o "b main" -o r -o q -b %t.exe | FileCheck %s
 



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


[Lldb-commits] [PATCH] D80308: [lldb] Enable C++14 when evaluating expressions in a C++14 frame

2020-05-22 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments.



Comment at: lldb/test/API/lang/cpp/standards/cpp14/TestCPP14Standard.py:19
+# polymorphic lambdas).
+self.expect_expr("[](auto x) { return x; }(1)", result_type="int", 
result_value="1")

teemperor wrote:
> shafik wrote:
> > It would be worth it to add a more complete set of C++14 tests. 
> > [p1319r0](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1319r0.html)
> >  covers the C++11 to C++14 differences. 
> > 
> > Especially ones that may not work so we can file bugs for these and track 
> > them.
> We're only checking that the right Clang language standard was set, not 
> Clang's C++ features related to that set standard (which should be and 
> probably already is a test in Clang).
Well, yes and no. We have seen features that should work because clang supports 
it but don't because of other reasons such as variadic templates and structured 
bindings. So just because clang supports it does not mean it won't break in 
expressions because of some other reason. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80308



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


[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-22 Thread Djordje Todorovic via Phabricator via lldb-commits
djtodoro added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:149
+EntryValue = 1 << 0,
+IndirectEntryValue = 1 << 1,
+CallSiteParamValue = 1 << 2

vsk wrote:
> aprantl wrote:
> > Would it make more sense call this `Indirect` (w/o EntryValue) and treat it 
> > as a separate bit, so you can still test for EntryValue independently?
> Yes, that does seem better. It's more extensible this way, and anyway, most 
> of the logic for treating direct/indirect entry values should be the same.
mot -> not



Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir:18
+# CHECK-NEXT: [0x, 0x0010): DW_OP_breg0 W0+0
+# CHECK-NEXT: [0x0010, 0x001c): 
DW_OP_entry_value(DW_OP_reg0 W0))
+# CHECK-NEXT: DW_AT_name("f")

vsk wrote:
> djtodoro wrote:
> > I know that the final effect is the same, but should this be 
> > `DW_OP_entry_value 2 DW_OP_bregN 0` instead of `DW_OP_entry_value 1 
> > DW_OP_regN`?
> I'm not sure, the DW_OP_regN description is simply the one that has already 
> been wired up. Is there a reason to prefer 'DW_OP_bregN 0'?
I was thinking about a more complex entry values where the offset is other than 
`0`, where by using this way of representation we'll have more complex 
expression for the same thing.
If we have expressions as:

1) `DW_OP_entry_value 3 DW_OP_bregN M DW_OP_deref DW_OP_stack_value`

2) `DW_OP_entry_value 6 DW_OP_entry_value 1 DW_OP_regN DW_OP_plus_uconst M 
DW_OP_deref DW_OP_stack_value`

The two expressions have the same effect. They push the value memory location 
pointed to by value of  register `N` upon entering current subprogram plus `M` 
had upon entering of the current subprogram.

This may not be worth of considering now, but we can put a TODO marker in 
`DwarfExpression`.

(Sorry If I am asking to much) One quick question:

Should the `DW_OP_entry_value(DW_OP_reg0 W0))` be an entry value expression 
with `dw_op_deref` + `dw_op_stack_val`?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345



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


[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-22 Thread Djordje Todorovic via Phabricator via lldb-commits
djtodoro added a comment.

Great! Thanks!

I think we should update the `LangRef.rst` (entry_values section) as well.

In addition, can we add a test case checking MIR output after `LiveDebugValues`?




Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir:18
+# CHECK-NEXT: [0x, 0x0010): DW_OP_breg0 W0+0
+# CHECK-NEXT: [0x0010, 0x001c): 
DW_OP_entry_value(DW_OP_reg0 W0))
+# CHECK-NEXT: DW_AT_name("f")

I know that the final effect is the same, but should this be `DW_OP_entry_value 
2 DW_OP_bregN 0` instead of `DW_OP_entry_value 1 DW_OP_regN`?



Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir:71
+  !24 = !DILocation(line: 5, column: 24, scope: !13)
+  !25 = !DILocation(line: 6, column: 23, scope: !13)
+  !31 = !DILocation(line: 6, column: 20, scope: !13)

Nit: !25-36! could be attached to !24



Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir:79
+---
+name:bar
+alignment:   4

I believe we do need all the attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345



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


[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-22 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:151
+IndirectValue = 1 << 1,
+CallSiteParamValue = 1 << 2
+  };

I'm going to be pedantic now: Should this be Indirect instead of IndirectValue? 
I.e., can there be non-values (= modifyable locations) that are indirect?



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:162
+  void adjustLocationKind(const MachineLocation &Loc,
+  const DIExpression *DIExpr);
+

"adjust" sounds like you could call this more than once. Should it be "set" or 
"initialize"? Or even be the constructor?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345



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


[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-22 Thread Vedant Kumar via Phabricator via lldb-commits
vsk updated this revision to Diff 265594.
vsk marked an inline comment as done.
vsk added a comment.

Address review feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345

Files:
  lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
  llvm/lib/CodeGen/LiveDebugValues.cpp
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir

Index: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir
===
--- /dev/null
+++ llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir
@@ -0,0 +1,109 @@
+# RUN: llc -emit-call-site-info -start-before=livedebugvalues -filetype=obj -o - %s \
+# RUN:   | llvm-dwarfdump - | FileCheck %s -implicit-check-not=DW_OP_entry_value
+
+# // Original Source
+# struct fat_ptr {
+#   int *ptr, *low, *high;
+# };
+# extern int baz(int x);
+# int bar(struct fat_ptr f) {
+#   return baz(baz(*f.ptr));
+# }
+
+# After w0 is clobbered, we should get an indirect parameter entry value for "f".
+
+# CHECK-LABEL: DW_TAG_formal_parameter
+# CHECK-NEXT: DW_AT_location
+# CHECK-NEXT: [0x, 0x0010): DW_OP_breg0 W0+0
+# CHECK-NEXT: [0x0010, 0x001c): DW_OP_entry_value(DW_OP_reg0 W0))
+# CHECK-NEXT: DW_AT_name("f")
+
+--- |
+  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+  target triple = "arm64-apple-ios10.0.0"
+  
+  %struct.fat_ptr = type { i32*, i32*, i32* }
+  
+  define i32 @bar(%struct.fat_ptr* nocapture readonly %f) local_unnamed_addr !dbg !13 {
+  entry:
+call void @llvm.dbg.declare(metadata %struct.fat_ptr* %f, metadata !23, metadata !DIExpression()), !dbg !24
+%ptr2 = bitcast %struct.fat_ptr* %f to i32**, !dbg !25
+%0 = load i32*, i32** %ptr2, align 8, !dbg !25
+%1 = load i32, i32* %0, align 4, !dbg !31
+%call = tail call i32 @baz(i32 %1), !dbg !34
+%call1 = tail call i32 @baz(i32 %call), !dbg !35
+ret i32 %call1, !dbg !36
+  }
+  
+  declare void @llvm.dbg.declare(metadata, metadata, metadata)
+  
+  declare !dbg !4 i32 @baz(i32) local_unnamed_addr optsize
+  
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!8, !9, !10, !11}
+  !llvm.ident = !{!12}
+  
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None, sysroot: "/")
+  !1 = !DIFile(filename: "indirect.c", directory: "/tmp/fatptr")
+  !2 = !{}
+  !3 = !{!4}
+  !4 = !DISubprogram(name: "baz", scope: !1, file: !1, line: 4, type: !5, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+  !5 = !DISubroutineType(types: !6)
+  !6 = !{!7, !7}
+  !7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !8 = !{i32 7, !"Dwarf Version", i32 4}
+  !9 = !{i32 2, !"Debug Info Version", i32 3}
+  !10 = !{i32 1, !"wchar_size", i32 4}
+  !11 = !{i32 7, !"PIC Level", i32 2}
+  !12 = !{!"clang"}
+  !13 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 5, type: !14, scopeLine: 5, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !22)
+  !14 = !DISubroutineType(types: !15)
+  !15 = !{!7, !16}
+  !16 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "fat_ptr", file: !1, line: 1, size: 192, elements: !17)
+  !17 = !{!18, !20, !21}
+  !18 = !DIDerivedType(tag: DW_TAG_member, name: "ptr", scope: !16, file: !1, line: 2, baseType: !19, size: 64)
+  !19 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !7, size: 64)
+  !20 = !DIDerivedType(tag: DW_TAG_member, name: "low", scope: !16, file: !1, line: 2, baseType: !19, size: 64, offset: 64)
+  !21 = !DIDerivedType(tag: DW_TAG_member, name: "high", scope: !16, file: !1, line: 2, baseType: !19, size: 64, offset: 128)
+  !22 = !{!23}
+  !23 = !DILocalVariable(name: "f", arg: 1, scope: !13, file: !1, line: 5, type: !16)
+  !24 = !DILocation(line: 5, column: 24, scope: !13)
+  !25 = !DILocation(line: 6, column: 23, scope: !13)
+  !31 = !DILocation(line: 6, column: 20, scope: !13)
+  !34 = !DILocation(line: 6, column: 16, scope: !13)
+  !35 = !DILocation(line: 6, column: 12, scope: !13)
+  !36 = !DILocation(line: 6, column: 5, scope: !13)
+
+...
+---
+name:bar
+stack:
+  - { id: 0, name: '', type: spill-slot, offset: -8, size: 8, alignment: 8, 
+  stack-id: default, callee-saved-register: '$lr', callee-saved-restored: true, 
+  debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+  - { id: 1, name: '', type: spill-slot, offset: -16, size: 8, alignment: 8, 
+  stack-id: default, callee-saved-register: '$fp', callee-sav

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-22 Thread Vedant Kumar via Phabricator via lldb-commits
vsk updated this revision to Diff 265629.
vsk added a comment.

Remove incorrect fixme.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345

Files:
  lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
  llvm/lib/CodeGen/LiveDebugValues.cpp
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir

Index: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir
===
--- /dev/null
+++ llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir
@@ -0,0 +1,109 @@
+# RUN: llc -emit-call-site-info -start-before=livedebugvalues -filetype=obj -o - %s \
+# RUN:   | llvm-dwarfdump - | FileCheck %s -implicit-check-not=DW_OP_entry_value
+
+# // Original Source
+# struct fat_ptr {
+#   int *ptr, *low, *high;
+# };
+# extern int baz(int x);
+# int bar(struct fat_ptr f) {
+#   return baz(baz(*f.ptr));
+# }
+
+# After w0 is clobbered, we should get an indirect parameter entry value for "f".
+
+# CHECK-LABEL: DW_TAG_formal_parameter
+# CHECK-NEXT: DW_AT_location
+# CHECK-NEXT: [0x, 0x0010): DW_OP_breg0 W0+0
+# CHECK-NEXT: [0x0010, 0x001c): DW_OP_entry_value(DW_OP_reg0 W0))
+# CHECK-NEXT: DW_AT_name("f")
+
+--- |
+  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+  target triple = "arm64-apple-ios10.0.0"
+  
+  %struct.fat_ptr = type { i32*, i32*, i32* }
+  
+  define i32 @bar(%struct.fat_ptr* nocapture readonly %f) local_unnamed_addr !dbg !13 {
+  entry:
+call void @llvm.dbg.declare(metadata %struct.fat_ptr* %f, metadata !23, metadata !DIExpression()), !dbg !24
+%ptr2 = bitcast %struct.fat_ptr* %f to i32**, !dbg !25
+%0 = load i32*, i32** %ptr2, align 8, !dbg !25
+%1 = load i32, i32* %0, align 4, !dbg !31
+%call = tail call i32 @baz(i32 %1), !dbg !34
+%call1 = tail call i32 @baz(i32 %call), !dbg !35
+ret i32 %call1, !dbg !36
+  }
+  
+  declare void @llvm.dbg.declare(metadata, metadata, metadata)
+  
+  declare !dbg !4 i32 @baz(i32) local_unnamed_addr optsize
+  
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!8, !9, !10, !11}
+  !llvm.ident = !{!12}
+  
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None, sysroot: "/")
+  !1 = !DIFile(filename: "indirect.c", directory: "/tmp/fatptr")
+  !2 = !{}
+  !3 = !{!4}
+  !4 = !DISubprogram(name: "baz", scope: !1, file: !1, line: 4, type: !5, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+  !5 = !DISubroutineType(types: !6)
+  !6 = !{!7, !7}
+  !7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !8 = !{i32 7, !"Dwarf Version", i32 4}
+  !9 = !{i32 2, !"Debug Info Version", i32 3}
+  !10 = !{i32 1, !"wchar_size", i32 4}
+  !11 = !{i32 7, !"PIC Level", i32 2}
+  !12 = !{!"clang"}
+  !13 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 5, type: !14, scopeLine: 5, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !22)
+  !14 = !DISubroutineType(types: !15)
+  !15 = !{!7, !16}
+  !16 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "fat_ptr", file: !1, line: 1, size: 192, elements: !17)
+  !17 = !{!18, !20, !21}
+  !18 = !DIDerivedType(tag: DW_TAG_member, name: "ptr", scope: !16, file: !1, line: 2, baseType: !19, size: 64)
+  !19 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !7, size: 64)
+  !20 = !DIDerivedType(tag: DW_TAG_member, name: "low", scope: !16, file: !1, line: 2, baseType: !19, size: 64, offset: 64)
+  !21 = !DIDerivedType(tag: DW_TAG_member, name: "high", scope: !16, file: !1, line: 2, baseType: !19, size: 64, offset: 128)
+  !22 = !{!23}
+  !23 = !DILocalVariable(name: "f", arg: 1, scope: !13, file: !1, line: 5, type: !16)
+  !24 = !DILocation(line: 5, column: 24, scope: !13)
+  !25 = !DILocation(line: 6, column: 23, scope: !13)
+  !31 = !DILocation(line: 6, column: 20, scope: !13)
+  !34 = !DILocation(line: 6, column: 16, scope: !13)
+  !35 = !DILocation(line: 6, column: 12, scope: !13)
+  !36 = !DILocation(line: 6, column: 5, scope: !13)
+
+...
+---
+name:bar
+stack:
+  - { id: 0, name: '', type: spill-slot, offset: -8, size: 8, alignment: 8, 
+  stack-id: default, callee-saved-register: '$lr', callee-saved-restored: true, 
+  debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+  - { id: 1, name: '', type: spill-slot, offset: -16, size: 8, alignment: 8, 
+  stack-id: default, callee-saved-register: '$fp', callee-saved-restored: true, 
+  debug-info-v

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-22 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:1288
   DwarfExpr.addFragmentOffset(DIExpr);
-  if (Location.isIndirect())
+  if (Location.isIndirect() && !DIExpr->isEntryValue())
 DwarfExpr.setMemoryLocationKind();

Can you add a comment? it's not necessarily obvious why an indirect entry value 
is not a memory location. Is it because an entry value is its own kind?



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:790
+// FIXME: This produces unusable descriptions when the register contains
+// a pointer to a temporary copy of a struct passed by value.
 DIExpression *EntryExpr = DIExpression::get(

What does "unusable" mean? Incorrect? Invalid?



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2404
 MachineLocation Location = Value.getLoc();
-if (Location.isIndirect())
+if (Location.isIndirect() && !DIExpr->isEntryValue())
   DwarfExpr.setMemoryLocationKind();

same here



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2412
 }
 
 const TargetRegisterInfo &TRI = *AP.MF->getSubtarget().getRegisterInfo();

factoring out this snippet into a template and reusing it in 
DwarfCompileUnit.cpp is probably not going to make this more readable, is it?



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:146
 
   /// The flags of location description being produced.
+  enum {

This comment is weird :-)

`Additional flags that may be combined with any location description kind.`?



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:149
+EntryValue = 1 << 0,
+IndirectEntryValue = 1 << 1,
+CallSiteParamValue = 1 << 2

Would it make more sense call this `Indirect` (w/o EntryValue) and treat it as 
a separate bit, so you can still test for EntryValue independently?



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:308
   /// Lock this down to become an entry value location.
-  void setEntryValueFlag() { LocationFlags |= EntryValue; }
+  void setEntryValueFlagFromLoc(MachineLocation Loc) {
+LocationFlags |= Loc.isIndirect() ? IndirectEntryValue : EntryValue;

Is the dependency on MachineLocation really necessary, or could this just take 
a bool / enum IsIndirect?



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:309
+  void setEntryValueFlagFromLoc(MachineLocation Loc) {
+LocationFlags |= Loc.isIndirect() ? IndirectEntryValue : EntryValue;
+  }

see comment in header...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345



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


[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-22 Thread Vedant Kumar via Phabricator via lldb-commits
vsk marked 2 inline comments as done.
vsk added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:151
+IndirectValue = 1 << 1,
+CallSiteParamValue = 1 << 2
+  };

aprantl wrote:
> I'm going to be pedantic now: Should this be Indirect instead of 
> IndirectValue? I.e., can there be non-values (= modifyable locations) that 
> are indirect?
Oh, I wasn't aware "Value" connoted "non-modifiable". In that case, "Indirect" 
is certainly a better fit: the user can choose to modify the temporary slot in 
the caller for the indirect param if they'd like.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:162
+  void adjustLocationKind(const MachineLocation &Loc,
+  const DIExpression *DIExpr);
+

aprantl wrote:
> "adjust" sounds like you could call this more than once. Should it be "set" 
> or "initialize"? Or even be the constructor?
"set" sounds good. The location isn't always available when the DwarfExpression 
is constructed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345



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


[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-22 Thread Vedant Kumar via Phabricator via lldb-commits
vsk marked 2 inline comments as done.
vsk added inline comments.



Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir:18
+# CHECK-NEXT: [0x, 0x0010): DW_OP_breg0 W0+0
+# CHECK-NEXT: [0x0010, 0x001c): 
DW_OP_entry_value(DW_OP_reg0 W0))
+# CHECK-NEXT: DW_AT_name("f")

djtodoro wrote:
> vsk wrote:
> > djtodoro wrote:
> > > I know that the final effect is the same, but should this be 
> > > `DW_OP_entry_value 2 DW_OP_bregN 0` instead of `DW_OP_entry_value 1 
> > > DW_OP_regN`?
> > I'm not sure, the DW_OP_regN description is simply the one that has already 
> > been wired up. Is there a reason to prefer 'DW_OP_bregN 0'?
> I was thinking about a more complex entry values where the offset is other 
> than `0`, where by using this way of representation we'll have more complex 
> expression for the same thing.
> If we have expressions as:
> 
> 1) `DW_OP_entry_value 3 DW_OP_bregN M DW_OP_deref DW_OP_stack_value`
> 
> 2) `DW_OP_entry_value 6 DW_OP_entry_value 1 DW_OP_regN DW_OP_plus_uconst M 
> DW_OP_deref DW_OP_stack_value`
> 
> The two expressions have the same effect. They push the value memory location 
> pointed to by value of  register `N` upon entering current subprogram plus 
> `M` had upon entering of the current subprogram.
> 
> This may not be worth of considering now, but we can put a TODO marker in 
> `DwarfExpression`.
> 
> (Sorry If I am asking to much) One quick question:
> 
> Should the `DW_OP_entry_value(DW_OP_reg0 W0))` be an entry value expression 
> with `dw_op_deref` + `dw_op_stack_val`?
> 
These are good questions.

If the indirect parameter offset is not 0, then it seems perfectly fine to 
apply it outside of the nested entry value expression. However, we don't 
support emitting entry values in this case. I've added a test to illustrate 
that.

As for DW_OP_stack_value, I'm not sure it's the right thing to use for 
locations. It breaks LLDB, fwiw: LLDB doesn't have the type of the object 
available as it's evaluating a DWARF expression, so when it sees the 
DW_OP_deref, it can't figure out the size and fails to reconstruct the object.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345



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


[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-22 Thread Vedant Kumar via Phabricator via lldb-commits
vsk updated this revision to Diff 265805.
vsk added a comment.

Add test coverage for indirect parameter location with offset; fix typo; fix 
LangRef entry.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345

Files:
  lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp
  llvm/docs/LangRef.rst
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
  llvm/lib/CodeGen/LiveDebugValues.cpp
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param-with-offset.mir
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir

Index: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir
===
--- /dev/null
+++ llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir
@@ -0,0 +1,117 @@
+# RUN: llc -emit-call-site-info -start-before=livedebugvalues -stop-after=machineverifier -o - %s \
+# RUN:   | FileCheck %s -check-prefix=MIR
+
+# RUN: llc -emit-call-site-info -start-before=livedebugvalues -filetype=obj -o - %s \
+# RUN:   | llvm-dwarfdump - | FileCheck %s -check-prefix=DWARF -implicit-check-not=DW_OP_entry_value
+
+# // Original Source
+# struct fat_ptr {
+#   int *ptr, *low, *high;
+# };
+# extern int baz(int x);
+# int bar(struct fat_ptr f) {
+#   return baz(baz(*f.ptr));
+# }
+
+# MIR:  renamable $w0 = LDRWui killed renamable $x8
+# MIR-NEXT: DBG_VALUE $x0, 0, {{.*}}, !DIExpression(DW_OP_LLVM_entry_value, 1)
+# MIR-NEXT: BL @baz
+# MIR-NEXT: frame-destroy LDPXpost
+# MIR-NEXT: TCRETURNdi @baz
+
+# After w0 is clobbered, we should get an indirect parameter entry value for "f".
+
+# DWARF-LABEL: DW_TAG_formal_parameter
+# DWARF-NEXT: DW_AT_location
+# DWARF-NEXT: [0x, 0x0010): DW_OP_breg0 W0+0
+# DWARF-NEXT: [0x0010, 0x001c): DW_OP_entry_value(DW_OP_reg0 W0))
+# DWARF-NEXT: DW_AT_name("f")
+
+--- |
+  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+  target triple = "arm64-apple-ios10.0.0"
+
+  %struct.fat_ptr = type { i32*, i32*, i32* }
+
+  define i32 @bar(%struct.fat_ptr* nocapture readonly %f) local_unnamed_addr !dbg !13 {
+  entry:
+call void @llvm.dbg.declare(metadata %struct.fat_ptr* %f, metadata !23, metadata !DIExpression()), !dbg !24
+%ptr2 = bitcast %struct.fat_ptr* %f to i32**, !dbg !25
+%0 = load i32*, i32** %ptr2, align 8, !dbg !25
+%1 = load i32, i32* %0, align 4, !dbg !31
+%call = tail call i32 @baz(i32 %1), !dbg !34
+%call1 = tail call i32 @baz(i32 %call), !dbg !35
+ret i32 %call1, !dbg !36
+  }
+
+  declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+  declare !dbg !4 i32 @baz(i32) local_unnamed_addr optsize
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!8, !9, !10, !11}
+  !llvm.ident = !{!12}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None, sysroot: "/")
+  !1 = !DIFile(filename: "indirect.c", directory: "/tmp/fatptr")
+  !2 = !{}
+  !3 = !{!4}
+  !4 = !DISubprogram(name: "baz", scope: !1, file: !1, line: 4, type: !5, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+  !5 = !DISubroutineType(types: !6)
+  !6 = !{!7, !7}
+  !7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !8 = !{i32 7, !"Dwarf Version", i32 4}
+  !9 = !{i32 2, !"Debug Info Version", i32 3}
+  !10 = !{i32 1, !"wchar_size", i32 4}
+  !11 = !{i32 7, !"PIC Level", i32 2}
+  !12 = !{!"clang"}
+  !13 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 5, type: !14, scopeLine: 5, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !22)
+  !14 = !DISubroutineType(types: !15)
+  !15 = !{!7, !16}
+  !16 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "fat_ptr", file: !1, line: 1, size: 192, elements: !17)
+  !17 = !{!18, !20, !21}
+  !18 = !DIDerivedType(tag: DW_TAG_member, name: "ptr", scope: !16, file: !1, line: 2, baseType: !19, size: 64)
+  !19 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !7, size: 64)
+  !20 = !DIDerivedType(tag: DW_TAG_member, name: "low", scope: !16, file: !1, line: 2, baseType: !19, size: 64, offset: 64)
+  !21 = !DIDerivedType(tag: DW_TAG_member, name: "high", scope: !16, file: !1, line: 2, baseType: !19, size: 64, offset: 128)
+  !22 = !{!23}
+  !23 = !DILocalVariable(name: "f", arg: 1, scope: !13, file: !1, line: 5, type: !16)
+  !24 = !DILocation(line: 5, column: 24, scope: !13)
+  !25 = !DILocation(line: 6, column: 23, scope: !13)
+  !31 = !DILocation(line: 6, column: 20, scope: !13)
+  !34 = !DILocation(line: 6, column: 16, scope: !13)
+  !35 = !DILocation(line: 6, co

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-22 Thread Vedant Kumar via Phabricator via lldb-commits
vsk marked 11 inline comments as done.
vsk added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:1288
   DwarfExpr.addFragmentOffset(DIExpr);
-  if (Location.isIndirect())
+  if (Location.isIndirect() && !DIExpr->isEntryValue())
 DwarfExpr.setMemoryLocationKind();

aprantl wrote:
> Can you add a comment? it's not necessarily obvious why an indirect entry 
> value is not a memory location. Is it because an entry value is its own kind?
This has more to do with pre-existing constraints in the dwarf emitter, more 
than anything else. `DwarfExpression::addReg` cannot be used with Memory 
location kinds, but we need that method to write out 
DW_OP_entry_value(DW_OP_regN).

I guess there's no such restriction for `addBReg`, but I'm not sure why we'd 
pick that representation (see Djordje's earlier comment).



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:790
+// FIXME: This produces unusable descriptions when the register contains
+// a pointer to a temporary copy of a struct passed by value.
 DIExpression *EntryExpr = DIExpression::get(

aprantl wrote:
> What does "unusable" mean? Incorrect? Invalid?
Should be "incorrect", fixed. It's "unusable" in the sense that the debugger 
will error out trying to evaluate the expression: it will think, "struct Foo 
/is/ the pointer in this register", instead of, "struct Foo is /pointed to/ by 
this register".



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2412
 }
 
 const TargetRegisterInfo &TRI = *AP.MF->getSubtarget().getRegisterInfo();

aprantl wrote:
> factoring out this snippet into a template and reusing it in 
> DwarfCompileUnit.cpp is probably not going to make this more readable, is it?
I tried doing this a few different ways :). I couldn't find a clean solution. 
In the end I opted for adding 'DwarfExpression::adjustLocationKind' to share 
just some of the logic. Lmk what you think.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:146
 
   /// The flags of location description being produced.
+  enum {

aprantl wrote:
> This comment is weird :-)
> 
> `Additional flags that may be combined with any location description kind.`?
Fixed, that's much better.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:149
+EntryValue = 1 << 0,
+IndirectEntryValue = 1 << 1,
+CallSiteParamValue = 1 << 2

aprantl wrote:
> Would it make more sense call this `Indirect` (w/o EntryValue) and treat it 
> as a separate bit, so you can still test for EntryValue independently?
Yes, that does seem better. It's more extensible this way, and anyway, most of 
the logic for treating direct/indirect entry values should be the same.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:308
   /// Lock this down to become an entry value location.
-  void setEntryValueFlag() { LocationFlags |= EntryValue; }
+  void setEntryValueFlagFromLoc(MachineLocation Loc) {
+LocationFlags |= Loc.isIndirect() ? IndirectEntryValue : EntryValue;

aprantl wrote:
> Is the dependency on MachineLocation really necessary, or could this just 
> take a bool / enum IsIndirect?
It aids encapsulation to not require clients to unpack the relevant bit of 
MachineLocation, I feel. I've changed this to accept a reference, to at least 
get rid of the header dependency. This seems acceptable to me, as 
DwarfExpression clients work with MachineLocation anyway.



Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir:18
+# CHECK-NEXT: [0x, 0x0010): DW_OP_breg0 W0+0
+# CHECK-NEXT: [0x0010, 0x001c): 
DW_OP_entry_value(DW_OP_reg0 W0))
+# CHECK-NEXT: DW_AT_name("f")

djtodoro wrote:
> I know that the final effect is the same, but should this be 
> `DW_OP_entry_value 2 DW_OP_bregN 0` instead of `DW_OP_entry_value 1 
> DW_OP_regN`?
I'm not sure, the DW_OP_regN description is simply the one that has already 
been wired up. Is there a reason to prefer 'DW_OP_bregN 0'?



Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir:71
+  !24 = !DILocation(line: 5, column: 24, scope: !13)
+  !25 = !DILocation(line: 6, column: 23, scope: !13)
+  !31 = !DILocation(line: 6, column: 20, scope: !13)

djtodoro wrote:
> Nit: !25-36! could be attached to !24
That's a fair point, but I feel this is too onerous of a reduction to do by 
hand. As the MIR is already fairly reduced, I would prefer not to do this.



Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir:79
+---
+name:bar
+alignment:   4

djtodoro wrote:
> I believe we do need all the attributes.
Fixed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE L

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-22 Thread Vedant Kumar via Phabricator via lldb-commits
vsk marked an inline comment as done.
vsk added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:790
+// FIXME: This produces unusable descriptions when the register contains
+// a pointer to a temporary copy of a struct passed by value.
 DIExpression *EntryExpr = DIExpression::get(

vsk wrote:
> aprantl wrote:
> > What does "unusable" mean? Incorrect? Invalid?
> Should be "incorrect", fixed. It's "unusable" in the sense that the debugger 
> will error out trying to evaluate the expression: it will think, "struct Foo 
> /is/ the pointer in this register", instead of, "struct Foo is /pointed to/ 
> by this register".
Actually, this FIXME isn't correct. This is a holdover from an alternative 
implementation I had which introduced a DW_OP_LLVM_indirect_entry_value opcode. 
I'll remove this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345



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


[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-22 Thread Vedant Kumar via Phabricator via lldb-commits
vsk updated this revision to Diff 265815.
vsk added a comment.

Address review feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345

Files:
  lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp
  llvm/docs/LangRef.rst
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
  llvm/lib/CodeGen/LiveDebugValues.cpp
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param-with-offset.mir
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir

Index: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir
===
--- /dev/null
+++ llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir
@@ -0,0 +1,117 @@
+# RUN: llc -emit-call-site-info -start-before=livedebugvalues -stop-after=machineverifier -o - %s \
+# RUN:   | FileCheck %s -check-prefix=MIR
+
+# RUN: llc -emit-call-site-info -start-before=livedebugvalues -filetype=obj -o - %s \
+# RUN:   | llvm-dwarfdump - | FileCheck %s -check-prefix=DWARF -implicit-check-not=DW_OP_entry_value
+
+# // Original Source
+# struct fat_ptr {
+#   int *ptr, *low, *high;
+# };
+# extern int baz(int x);
+# int bar(struct fat_ptr f) {
+#   return baz(baz(*f.ptr));
+# }
+
+# MIR:  renamable $w0 = LDRWui killed renamable $x8
+# MIR-NEXT: DBG_VALUE $x0, 0, {{.*}}, !DIExpression(DW_OP_LLVM_entry_value, 1)
+# MIR-NEXT: BL @baz
+# MIR-NEXT: frame-destroy LDPXpost
+# MIR-NEXT: TCRETURNdi @baz
+
+# After w0 is clobbered, we should get an indirect parameter entry value for "f".
+
+# DWARF-LABEL: DW_TAG_formal_parameter
+# DWARF-NEXT: DW_AT_location
+# DWARF-NEXT: [0x, 0x0010): DW_OP_breg0 W0+0
+# DWARF-NEXT: [0x0010, 0x001c): DW_OP_entry_value(DW_OP_reg0 W0))
+# DWARF-NEXT: DW_AT_name("f")
+
+--- |
+  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+  target triple = "arm64-apple-ios10.0.0"
+
+  %struct.fat_ptr = type { i32*, i32*, i32* }
+
+  define i32 @bar(%struct.fat_ptr* nocapture readonly %f) local_unnamed_addr !dbg !13 {
+  entry:
+call void @llvm.dbg.declare(metadata %struct.fat_ptr* %f, metadata !23, metadata !DIExpression()), !dbg !24
+%ptr2 = bitcast %struct.fat_ptr* %f to i32**, !dbg !25
+%0 = load i32*, i32** %ptr2, align 8, !dbg !25
+%1 = load i32, i32* %0, align 4, !dbg !31
+%call = tail call i32 @baz(i32 %1), !dbg !34
+%call1 = tail call i32 @baz(i32 %call), !dbg !35
+ret i32 %call1, !dbg !36
+  }
+
+  declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+  declare !dbg !4 i32 @baz(i32) local_unnamed_addr optsize
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!8, !9, !10, !11}
+  !llvm.ident = !{!12}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None, sysroot: "/")
+  !1 = !DIFile(filename: "indirect.c", directory: "/tmp/fatptr")
+  !2 = !{}
+  !3 = !{!4}
+  !4 = !DISubprogram(name: "baz", scope: !1, file: !1, line: 4, type: !5, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+  !5 = !DISubroutineType(types: !6)
+  !6 = !{!7, !7}
+  !7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !8 = !{i32 7, !"Dwarf Version", i32 4}
+  !9 = !{i32 2, !"Debug Info Version", i32 3}
+  !10 = !{i32 1, !"wchar_size", i32 4}
+  !11 = !{i32 7, !"PIC Level", i32 2}
+  !12 = !{!"clang"}
+  !13 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 5, type: !14, scopeLine: 5, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !22)
+  !14 = !DISubroutineType(types: !15)
+  !15 = !{!7, !16}
+  !16 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "fat_ptr", file: !1, line: 1, size: 192, elements: !17)
+  !17 = !{!18, !20, !21}
+  !18 = !DIDerivedType(tag: DW_TAG_member, name: "ptr", scope: !16, file: !1, line: 2, baseType: !19, size: 64)
+  !19 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !7, size: 64)
+  !20 = !DIDerivedType(tag: DW_TAG_member, name: "low", scope: !16, file: !1, line: 2, baseType: !19, size: 64, offset: 64)
+  !21 = !DIDerivedType(tag: DW_TAG_member, name: "high", scope: !16, file: !1, line: 2, baseType: !19, size: 64, offset: 128)
+  !22 = !{!23}
+  !23 = !DILocalVariable(name: "f", arg: 1, scope: !13, file: !1, line: 5, type: !16)
+  !24 = !DILocation(line: 5, column: 24, scope: !13)
+  !25 = !DILocation(line: 6, column: 23, scope: !13)
+  !31 = !DILocation(line: 6, column: 20, scope: !13)
+  !34 = !DILocation(line: 6, column: 16, scope: !13)
+  !35 = !DILocation(line: 6, column: 12, scope: !13)
+  !36 = !DILocation(line: 6, column: 5, scope

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

2020-05-22 Thread Vedant Kumar via Phabricator via lldb-commits
vsk added a comment.

Apparently, we always describe the value of an indirect parameter as being 
"whatever is in the temporary slot", even if the callee modifies it: 
https://godbolt.org/z/ZgWr_n. So treating the indirect parameter DBG_VALUE as 
pointing to a (modifiable) location sounds consistent to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345



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


[Lldb-commits] [PATCH] D80418: Print a warning when stopped in a frame LLDB has no plugin for.

2020-05-22 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl closed this revision.
aprantl added a comment.

220c17ffd4e1b127bcc02b25980b7934184ee1da 



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

https://reviews.llvm.org/D80418



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


Re: [Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

2020-05-22 Thread Vedant Kumar via lldb-commits
There were several things off here, both in the tests and the Cocoa.cpp change. 
Sorry for all the breakage.

I’ve updated https://reviews.llvm.org/D80150 
 to address these issues and written 
up a summary there of what went wrong.

vedant

> On May 20, 2020, at 9:23 AM, Eric Christopher  wrote:
> 
> Agreed. Something is off here. My change was only to silence a few warnings, 
> but they're definitely highlighting a conversion issue. What's up with NSDate 
> conversions here. Does the API have a way to convert from time_t?
> 
> On Wed, May 20, 2020, 2:07 AM Pavel Labath via Phabricator via lldb-commits 
> mailto:lldb-commits@lists.llvm.org>> wrote:
> labath added a comment.
> 
> In D80150#2045364  >, @vsk wrote:
> 
> > @labath Agreed on all points, I've addressed the feedback in 82dbf4aca84 
> >  > > by 
> > moving "DataFormatters/Mock.h" to "Plugins/Language/ObjC/Utilities.h", and 
> > adding a separate LanguageObjCTests unit test.
> 
> 
> Cool. Thanks.
> 
> 
> 
> 
> Comment at: lldb/unittests/DataFormatter/MockTests.cpp:30
> +  // Can't convert the date_value to a time_t.
> +  EXPECT_EQ(formatDateValue(std::numeric_limits::max() + 1),
> +llvm::None);
> 
> vsk wrote:
> > labath wrote:
> > > Isn't this actually `std::numeric_limits::min()` (and UB due to 
> > > singed wraparound) ? Did you want to convert to double before doing the 
> > > `+1` ?
> > Yes, thank you! It looks like Eric caught this before I did.
> Actually, thinking about that further, (for 64-bit `time_t`s), 
> `double(numeric_limits::max())` is [[ https://godbolt.org/z/t3iSd7 
>  | exactly the same value ]] as 
> `double(numeric_limits::max())+1.0` because `double` doesn't have 
> enough bits to represent the value precisely. So, I have a feeling these 
> checks are still not testing the exact thing you want to test (though I'm not 
> sure what that is exactly).
> 
> 
> Repository:
>   rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D80150/new/ 
> 
> https://reviews.llvm.org/D80150 
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org 
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits 
> 

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


[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

2020-05-22 Thread Vedant Kumar via Phabricator via lldb-commits
vsk updated this revision to Diff 265829.
vsk added a comment.

Apologies for all the breakage. I think I've identified the issues in the
initial patch which caused breakage on the bots, and which caused the tests to
not target what they meant to test.

I've reworked this patch to use std::llrint, instead of casting the max
`time_t` value to `double` as part of an overflow check. The cast to `double`
was not precise, and could crash on some machines (presumably due to a floating
point exception?).

For the tests, I've added asserts to make sure that the specially-crafted
values that are meant to trigger clamping behavior in std::llrint, or an
overflow in `llvm::checkedAdd`, actually do satisfy those conditions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80150

Files:
  lldb/include/lldb/DataFormatters/FormattersHelpers.h
  lldb/source/Plugins/Language/ObjC/CF.cpp
  lldb/source/Plugins/Language/ObjC/Cocoa.cpp
  lldb/source/Plugins/Language/ObjC/Utilities.h
  lldb/unittests/Language/AppleObjC/CMakeLists.txt
  lldb/unittests/Language/AppleObjC/UtilitiesTests.cpp
  lldb/unittests/Language/CMakeLists.txt

Index: lldb/unittests/Language/CMakeLists.txt
===
--- lldb/unittests/Language/CMakeLists.txt
+++ lldb/unittests/Language/CMakeLists.txt
@@ -1,2 +1,6 @@
 add_subdirectory(CPlusPlus)
 add_subdirectory(Highlighting)
+
+if (APPLE)
+  add_subdirectory(AppleObjC)
+endif()
Index: lldb/unittests/Language/AppleObjC/UtilitiesTests.cpp
===
--- /dev/null
+++ lldb/unittests/Language/AppleObjC/UtilitiesTests.cpp
@@ -0,0 +1,59 @@
+//===-- UtilitiesTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/Language/ObjC/Utilities.h"
+#include "lldb/Utility/StreamString.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+using namespace lldb_private;
+
+// Try to format the date_value field of a NSDate.
+static llvm::Optional formatDateValue(double date_value) {
+  StreamString s;
+  bool succcess = formatters::NSDate::FormatDateValue(date_value, s);
+  if (succcess)
+return std::string(s.GetData());
+  return llvm::None;
+}
+
+TEST(DataFormatterTest, NSDate) {
+  EXPECT_EQ(formatDateValue(-63114076800),
+std::string("0001-12-30 00:00:00 +"));
+
+  // Assert that std::llrint clamps values greater/lesser than the max/min
+  // time_t values. If it doesn't, it won't make sense to proceed, as we're
+  // relying on this behavior to avoid float-cast-overflow UB.
+  const double greater_than_max_time = 1e200;
+  const double lesser_than_min_time = -1e200;
+  ASSERT_LE(static_cast(std::llrint(greater_than_max_time)),
+std::numeric_limits::max());
+  ASSERT_LE(static_cast(std::llrint(lesser_than_min_time)),
+std::numeric_limits::min());
+
+  // If the date_value `double` cannot be represented in a `time_t`, give up.
+  EXPECT_EQ(formatDateValue(greater_than_max_time), llvm::None);
+  EXPECT_EQ(formatDateValue(lesser_than_min_time), llvm::None);
+
+  // If the date_value `double` plus the epoch cannot be represented in a
+  // `time_t`, give up. Try to craft a double that will trigger this scenario:
+  // if we can't do it, don't proceed.
+  const time_t date_value_to_trigger_overflow =
+  std::numeric_limits::max() - formatters::GetOSXEpoch() + 1;
+  const double date_value_to_trigger_overflow_as_double =
+  static_cast(date_value_to_trigger_overflow);
+  ASSERT_LE(date_value_to_trigger_overflow,
+static_cast(date_value_to_trigger_overflow_as_double));
+  EXPECT_EQ(formatDateValue(date_value_to_trigger_overflow), llvm::None);
+
+  EXPECT_EQ(formatDateValue(0), std::string("2001-01-01 00:00:00 UTC"));
+  EXPECT_EQ(formatDateValue(1024), std::string("2001-01-01 00:17:04 UTC"));
+}
Index: lldb/unittests/Language/AppleObjC/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Language/AppleObjC/CMakeLists.txt
@@ -0,0 +1,6 @@
+add_lldb_unittest(LanguageAppleObjCTests
+  UtilitiesTests.cpp
+
+  LINK_LIBS
+lldbPluginObjCLanguage
+  )
Index: lldb/source/Plugins/Language/ObjC/Utilities.h
===
--- /dev/null
+++ lldb/source/Plugins/Language/ObjC/Utilities.h
@@ -0,0 +1,32 @@
+//===-- Utilities.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.or

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

2020-05-22 Thread Vedant Kumar via Phabricator via lldb-commits
vsk marked 2 inline comments as done and an inline comment as not done.
vsk added inline comments.



Comment at: lldb/unittests/DataFormatter/MockTests.cpp:30
+  // Can't convert the date_value to a time_t.
+  EXPECT_EQ(formatDateValue(std::numeric_limits::max() + 1),
+llvm::None);

labath wrote:
> vsk wrote:
> > labath wrote:
> > > Isn't this actually `std::numeric_limits::min()` (and UB due to 
> > > singed wraparound) ? Did you want to convert to double before doing the 
> > > `+1` ?
> > Yes, thank you! It looks like Eric caught this before I did.
> Actually, thinking about that further, (for 64-bit `time_t`s), 
> `double(numeric_limits::max())` is [[ https://godbolt.org/z/t3iSd7 | 
> exactly the same value ]] as `double(numeric_limits::max())+1.0` 
> because `double` doesn't have enough bits to represent the value precisely. 
> So, I have a feeling these checks are still not testing the exact thing you 
> want to test (though I'm not sure what that is exactly).
Hm, that's right. What I'm trying to do is look for float-cast UB in two 
places: first, when we initially convert the `double` "date_value" to `time_t`, 
and second, when we add the OSXEpoch to that `time_t`.

I'll take a fresh cut at this. For the first conversion, I think I can rely on 
implementation-defined behavior from std::llrint to do the conversion without 
crashing LLDB due to a floating-point exception. For the second, we have 
`checkedAdd` from llvm, but the tricky part is testing it: to do that, I'm 
crafting a special `time_t` that should trigger overflow, and asserting that a 
round-trip conversion to/from `double` doesn't break the property.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80150



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


[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

2020-05-22 Thread Vedant Kumar via Phabricator via lldb-commits
vsk updated this revision to Diff 265830.
vsk added a comment.

Fix an ASSERT_LE that was actually meant to be an ASSERT_GE.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80150

Files:
  lldb/include/lldb/DataFormatters/FormattersHelpers.h
  lldb/source/Plugins/Language/ObjC/CF.cpp
  lldb/source/Plugins/Language/ObjC/Cocoa.cpp
  lldb/source/Plugins/Language/ObjC/Utilities.h
  lldb/unittests/Language/AppleObjC/CMakeLists.txt
  lldb/unittests/Language/AppleObjC/UtilitiesTests.cpp
  lldb/unittests/Language/CMakeLists.txt

Index: lldb/unittests/Language/CMakeLists.txt
===
--- lldb/unittests/Language/CMakeLists.txt
+++ lldb/unittests/Language/CMakeLists.txt
@@ -1,2 +1,6 @@
 add_subdirectory(CPlusPlus)
 add_subdirectory(Highlighting)
+
+if (APPLE)
+  add_subdirectory(AppleObjC)
+endif()
Index: lldb/unittests/Language/AppleObjC/UtilitiesTests.cpp
===
--- /dev/null
+++ lldb/unittests/Language/AppleObjC/UtilitiesTests.cpp
@@ -0,0 +1,60 @@
+//===-- UtilitiesTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/Language/ObjC/Utilities.h"
+#include "lldb/Utility/StreamString.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+using namespace lldb_private;
+
+// Try to format the date_value field of a NSDate.
+static llvm::Optional formatDateValue(double date_value) {
+  StreamString s;
+  bool succcess = formatters::NSDate::FormatDateValue(date_value, s);
+  if (succcess)
+return std::string(s.GetData());
+  return llvm::None;
+}
+
+TEST(DataFormatterTest, NSDate) {
+  EXPECT_EQ(formatDateValue(-63114076800),
+std::string("0001-12-30 00:00:00 +"));
+
+  // Assert that std::llrint clamps values greater/lesser than the max/min
+  // time_t values. If it doesn't, it won't make sense to proceed, as we're
+  // relying on this behavior to avoid float-cast-overflow UB.
+  const double greater_than_max_time = 1e200;
+  const double lesser_than_min_time = -1e200;
+  ASSERT_LE(static_cast(std::llrint(greater_than_max_time)),
+std::numeric_limits::max());
+  ASSERT_GE(static_cast(std::llrint(lesser_than_min_time)),
+std::numeric_limits::min());
+
+  // If the date_value `double` cannot be represented in a `time_t`, give up.
+  EXPECT_EQ(formatDateValue(greater_than_max_time), llvm::None);
+  EXPECT_EQ(formatDateValue(lesser_than_min_time), llvm::None);
+
+  // If the date_value `double` plus the epoch cannot be represented in a
+  // `time_t`, give up. Try to craft a double that will trigger this scenario:
+  // if we can't do it, don't proceed.
+  const time_t date_value_to_trigger_overflow =
+  std::numeric_limits::max() - formatters::GetOSXEpoch() + 1;
+  const double date_value_to_trigger_overflow_as_double =
+  static_cast(date_value_to_trigger_overflow);
+  ASSERT_LE(date_value_to_trigger_overflow,
+static_cast(date_value_to_trigger_overflow_as_double));
+  EXPECT_EQ(formatDateValue(date_value_to_trigger_overflow_as_double),
+llvm::None);
+
+  EXPECT_EQ(formatDateValue(0), std::string("2001-01-01 00:00:00 UTC"));
+  EXPECT_EQ(formatDateValue(1024), std::string("2001-01-01 00:17:04 UTC"));
+}
Index: lldb/unittests/Language/AppleObjC/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Language/AppleObjC/CMakeLists.txt
@@ -0,0 +1,6 @@
+add_lldb_unittest(LanguageAppleObjCTests
+  UtilitiesTests.cpp
+
+  LINK_LIBS
+lldbPluginObjCLanguage
+  )
Index: lldb/source/Plugins/Language/ObjC/Utilities.h
===
--- /dev/null
+++ lldb/source/Plugins/Language/ObjC/Utilities.h
@@ -0,0 +1,32 @@
+//===-- Utilities.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_PLUGINS_LANGUAGE_OBJC_UTILITIES_H
+#define LLDB_PLUGINS_LANGUAGE_OBJC_UTILITIES_H
+
+#include 
+
+namespace lldb_private {
+
+class Stream;
+
+namespace formatters {
+
+/// Return an epoch time_t adjusted to the reference date set by the Cocoa
+/// framework.
+time_t GetOSXEpoch();
+
+namespace NSDate {
+/// Format the date_value field of a NSDate.
+bool FormatDateValue

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

2020-05-22 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.

Major changes not required. Thanks for finding and fixing this!


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

https://reviews.llvm.org/D80254



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