[Lldb-commits] [lldb] 11b1eee - [lldb][NFC] Fix a variable name in ClangDiagnosticManagerAdapter

2020-07-02 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2020-07-02T09:10:07+02:00
New Revision: 11b1eeeaec642052e7299181c6a087f68807ae8b

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

LOG: [lldb][NFC] Fix a variable name in ClangDiagnosticManagerAdapter

Added: 


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 6b6346b69509..c8db6bb010f1 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -158,12 +158,12 @@ static void AddAllFixIts(ClangDiagnostic *diag, const 
clang::Diagnostic &Info) {
 class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
 public:
   ClangDiagnosticManagerAdapter(DiagnosticOptions &opts) {
-DiagnosticOptions *m_options = new DiagnosticOptions(opts);
-m_options->ShowPresumedLoc = true;
-m_options->ShowLevel = false;
+DiagnosticOptions *options = new DiagnosticOptions(opts);
+options->ShowPresumedLoc = true;
+options->ShowLevel = false;
 m_os = std::make_shared(m_output);
 m_passthrough =
-std::make_shared(*m_os, m_options);
+std::make_shared(*m_os, options);
   }
 
   void ResetManager(DiagnosticManager *manager = nullptr) {



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

[Lldb-commits] [lldb] 83aa58d - [lldb][NFC] Don't pass around passthrough from ClangDiagnosticManagerAdapter

2020-07-02 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2020-07-02T10:42:14+02:00
New Revision: 83aa58d795b92cd864c6c09d9a65817a14e63acc

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

LOG: [lldb][NFC] Don't pass around passthrough from 
ClangDiagnosticManagerAdapter

The passthrough DiagnosticConsumer is an implementation detail of
ClangDiagnosticManagerAdapter and we can just hide it behind the normal
DiagnosticConsumer interface that ClangDiagnosticManagerAdapter is supposed
to implement.

Added: 


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 c8db6bb010f1..6ff028cf6980 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -200,6 +200,9 @@ class ClangDiagnosticManagerAdapter : public 
clang::DiagnosticConsumer {
   return;
 }
 
+// Update error/warning counters.
+DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
+
 // Render diagnostic message to m_output.
 m_output.clear();
 m_passthrough->HandleDiagnostic(DiagLevel, Info);
@@ -261,7 +264,11 @@ class ClangDiagnosticManagerAdapter : public 
clang::DiagnosticConsumer {
 }
   }
 
-  clang::TextDiagnosticPrinter *GetPassthrough() { return m_passthrough.get(); 
}
+  void BeginSourceFile(const LangOptions &LO, const Preprocessor *PP) override 
{
+m_passthrough->BeginSourceFile(LO, PP);
+  }
+
+  void EndSourceFile() override { m_passthrough->EndSourceFile(); }
 
 private:
   DiagnosticManager *m_manager = nullptr;
@@ -967,7 +974,6 @@ ClangExpressionParser::ParseInternal(DiagnosticManager 
&diagnostic_manager,
   ClangDiagnosticManagerAdapter *adapter =
   static_cast(
   m_compiler->getDiagnostics().getClient());
-  auto diag_buf = adapter->GetPassthrough();
 
   adapter->ResetManager(&diagnostic_manager);
 
@@ -1022,8 +1028,8 @@ ClangExpressionParser::ParseInternal(DiagnosticManager 
&diagnostic_manager,
 
source_mgr.setMainFileID(source_mgr.createFileID(std::move(memory_buffer)));
   }
 
-  diag_buf->BeginSourceFile(m_compiler->getLangOpts(),
-&m_compiler->getPreprocessor());
+  adapter->BeginSourceFile(m_compiler->getLangOpts(),
+   &m_compiler->getPreprocessor());
 
   ClangExpressionHelper *type_system_helper =
   dyn_cast(m_expr.GetTypeSystemHelper());
@@ -1108,9 +1114,9 @@ ClangExpressionParser::ParseInternal(DiagnosticManager 
&diagnostic_manager,
   // original behavior of ParseAST (which also destroys the Sema after 
parsing).
   m_compiler->setSema(nullptr);
 
-  diag_buf->EndSourceFile();
+  adapter->EndSourceFile();
 
-  unsigned num_errors = diag_buf->getNumErrors();
+  unsigned num_errors = adapter->getNumErrors();
 
   if (m_pp_callbacks && m_pp_callbacks->hasErrors()) {
 num_errors++;



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

[Lldb-commits] [PATCH] D79699: Add ptrace register access for AArch64 SVE registers

2020-07-02 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid updated this revision to Diff 275038.
omjavaid added a comment.

This update remove dependence over generic interfaces by calling 
ConfigureRegisterContext on every call to ReadRegister/WriteRegister.


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

https://reviews.llvm.org/D79699

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c

Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
@@ -0,0 +1,5 @@
+int main() {
+  asm volatile("ptrue p0.s\n\t");
+  asm volatile("fcpy  z0.s, p0/m, #5.\n\t");
+  return 0; // Set a break point here.
+}
\ No newline at end of file
Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
@@ -0,0 +1,141 @@
+"""
+Test the AArch64 SVE registers.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class RegisterCommandsTestCase(TestBase):
+
+def check_sve_register_size(self, set, name, expected):
+reg_value = set.GetChildMemberWithName(name)
+self.assertTrue(reg_value.IsValid(),
+'Verify we have a register named "%s"' % (name))
+self.assertEqual(reg_value.GetByteSize(), expected,
+ 'Verify "%s" == %i' % (name, expected))
+
+mydir = TestBase.compute_mydir(__file__)
+@skipIf
+def test_sve_registers_configuration(self):
+"""Test AArch64 SVE registers size configuration."""
+self.build()
+self.line = line_number('main.c', '// Set a break point here.')
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1)
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint 1."])
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+thread = process.GetThreadAtIndex(0)
+currentFrame = thread.GetFrameAtIndex(0)
+
+has_sve = False
+for registerSet in currentFrame.GetRegisters():
+if 'sve registers' in registerSet.GetName().lower():
+has_sve = True
+
+if not has_sve:
+self.skipTest('SVE registers must be supported.')
+
+registerSets = process.GetThreadAtIndex(
+0).GetFrameAtIndex(0).GetRegisters()
+
+sve_registers = registerSets.GetValueAtIndex(2)
+
+vg_reg = sve_registers.GetChildMemberWithName("vg")
+
+vg_reg_value = sve_registers.GetChildMemberWithName(
+"vg").GetValueAsUnsigned()
+
+z_reg_size = vg_reg_value * 8
+
+p_reg_size = z_reg_size / 8
+
+for i in range(32):
+self.check_sve_register_size(
+sve_registers, 'z%i' % (i), z_reg_size)
+
+for i in range(16):
+self.check_sve_register_size(
+sve_registers, 'p%i' % (i), p_reg_size)
+
+self.check_sve_register_size(sve_registers, 'ffr', p_reg_size)
+
+mydir = TestBase.compute_mydir(__file__)
+@no_debug_info_test
+@skipIf
+def test_sve_registers_read_write(self):
+"""Test AArch64 SVE registers read and write."""
+self.build()
+self.line = line_number('main.c', '// Set a break point here.')
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1)
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint 1."])
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+thread = process.GetThreadAtIndex(0)
+currentFrame = thread.GetFrameAtIndex(0)
+
+has_sve = False
+

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and core file support

2020-07-02 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid updated this revision to Diff 275037.
omjavaid added a comment.

@labath I have updated diff after resolving issues highlighted. Also I have 
removed dependence on generic interfaces.


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

https://reviews.llvm.org/D77047

Files:
  lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64_sve.h
  lldb/source/Plugins/Process/Utility/lldb-arm64-register-enums.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  lldb/source/Utility/ARM64_DWARF_Registers.h
  lldb/source/Utility/ARM64_ehframe_Registers.h
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-sve.c
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-sve.core

Index: lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-sve.c
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-sve.c
@@ -0,0 +1,24 @@
+// compile with -march=armv8-a+sve on compatible aarch64 compiler
+// linux-aarch64-sve.core was generated by: aarch64-linux-gnu-gcc-8
+// commandline: -march=armv8-a+sve -nostdlib -static -g linux-aarch64-sve.c
+static void bar(char *boom) {
+  char F = 'b';
+  asm volatile("ptrue p0.s\n\t");
+  asm volatile("fcpy  z0.s, p0/m, #7.5\n\t");
+  asm volatile("ptrue p1.s\n\t");
+  asm volatile("fcpy  z1.s, p1/m, #11.5\n\t");
+  asm volatile("ptrue p3.s\n\t");
+  asm volatile("fcpy  z3.s, p3/m, #15.5\n\t");
+
+  *boom = 47; // Frame bar
+}
+
+static void foo(char *boom, void (*boomer)(char *)) {
+  char F = 'f';
+  boomer(boom); // Frame foo
+}
+
+void _start(void) {
+  char F = '_';
+  foo(0, bar); // Frame _start
+}
Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -269,7 +269,7 @@
 self.dbg.DeleteTarget(target)
 
 @skipIf(triple='^mips')
-@skipIfLLVMTargetMissing("AArch64")
+#@skipIfLLVMTargetMissing("AArch64")
 def test_aarch64_regs(self):
 # check 64 bit ARM core files
 target = self.dbg.CreateTarget(None)
@@ -323,6 +323,47 @@
 
 self.expect("register read --all")
 
+#@skipIf(triple='^mips')
+#@skipIfLLVMTargetMissing("AArch64")
+def test_aarch64_sve_regs(self):
+# check 64 bit ARM core files
+target = self.dbg.CreateTarget(None)
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore("linux-aarch64-sve.core")
+
+values = {}
+values["fp"] = "0xfc1ff4f0"
+values["lr"] = "0x00400170"
+values["sp"] = "0xfc1ff4d0"
+values["pc"] = "0x0040013c"
+values["v0"] = "{0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40}"
+values["v1"] = "{0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41}"
+values["v2"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+values["v3"] = "{0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41}"
+values["s0"] = "7.5"
+values["s1"] = "11.5"
+values["s2"] = "0"
+values["s3"] = "15.5"
+values["d0"] = "65536.0158538818"
+values["d1"] = "1572864.25476074"
+values["d2"] = "0"
+values["d3"] = "25165828.0917969"
+values["vg"] = "0x0004"
+values["z0"] = "{0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40}"
+values["z1"] = "{0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41}"
+values["z2"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+values["z3"] = "{0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41}"
+values["p0"] = "{0x11 0x11 0x11 0x11}"
+values["p1"] = "{0x11 0x11 0x11

[Lldb-commits] [lldb] d6343e6 - [lldb] Skip TestLimitDebugInfo on windows

2020-07-02 Thread Pavel Labath via lldb-commits 

Author: Pavel Labath
Date: 2020-07-02T14:34:33+02:00
New Revision: d6343e607ac8fa71fa6d99f9c86369ae9e66e671

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

LOG: [lldb] Skip TestLimitDebugInfo on windows

The test does not work on windows, because clang will emit full type
information for __declspec(dllexport) types even under
-flimit-debug-info. __declspec(dllexport) is needed to be able to use
the type across shared library boundaries on windows, which makes this a
pretty good heuristic, but defeats the purpose of this test.

I am going to create (in another patch) an basic assembly test, so that
the relevant code gets at least some coverage on windows hosts.

This also reverts commit 1276855f2b4485ec312b379c1b8eaf5510d9b157, which
added the __declspec annotations -- they are not necessary anymore, and
they needlessly complicate the test.

Added: 


Modified: 
lldb/test/API/functionalities/limit-debug-info/Makefile
lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
lldb/test/API/functionalities/limit-debug-info/onetwo.h

Removed: 




diff  --git a/lldb/test/API/functionalities/limit-debug-info/Makefile 
b/lldb/test/API/functionalities/limit-debug-info/Makefile
index 400f76e29e1a..874b3a15e0fe 100644
--- a/lldb/test/API/functionalities/limit-debug-info/Makefile
+++ b/lldb/test/API/functionalities/limit-debug-info/Makefile
@@ -2,12 +2,12 @@ CFLAGS_EXTRAS = $(LIMIT_DEBUG_INFO_FLAGS)
 LD_EXTRAS = -L. -lone -ltwo
 CXX_SOURCES = main.cpp
 
-ONE_CXXFLAGS = $(LIMIT_DEBUG_INFO_FLAGS) -DIN_ONE
+ONE_CXXFLAGS = $(LIMIT_DEBUG_INFO_FLAGS)
 ifdef STRIP_ONE
   ONE_CXXFLAGS += -g0
 endif
 
-TWO_CXXFLAGS = $(LIMIT_DEBUG_INFO_FLAGS) -DIN_TWO
+TWO_CXXFLAGS = $(LIMIT_DEBUG_INFO_FLAGS)
 ifdef STRIP_TWO
   TWO_CXXFLAGS += -g0
 endif

diff  --git 
a/lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py 
b/lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
index 48d32958388c..d22aeaace7bf 100644
--- a/lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
+++ b/lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
@@ -31,6 +31,7 @@ def _check_debug_info_is_limited(self, target):
 self._check_type(target, "InheritsFromTwo")
 
 @skipIf(bugnumber="pr46284", debug_info="gmodules")
+@skipIfWindows # Clang emits type info even with -flimit-debug-info
 def test_one_and_two_debug(self):
 self.build()
 target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
@@ -49,6 +50,7 @@ def test_one_and_two_debug(self):
 self.expect_expr("inherits_from_two.two", result_value="242")
 
 @skipIf(bugnumber="pr46284", debug_info="gmodules")
+@skipIfWindows # Clang emits type info even with -flimit-debug-info
 def test_two_debug(self):
 self.build(dictionary=dict(STRIP_ONE="1"))
 target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
@@ -68,6 +70,7 @@ def test_two_debug(self):
 self.expect_expr("inherits_from_two.two", result_value="242")
 
 @skipIf(bugnumber="pr46284", debug_info="gmodules")
+@skipIfWindows # Clang emits type info even with -flimit-debug-info
 def test_one_debug(self):
 self.build(dictionary=dict(STRIP_TWO="1"))
 target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))

diff  --git a/lldb/test/API/functionalities/limit-debug-info/onetwo.h 
b/lldb/test/API/functionalities/limit-debug-info/onetwo.h
index 602a88c58eeb..82df76c64b58 100644
--- a/lldb/test/API/functionalities/limit-debug-info/onetwo.h
+++ b/lldb/test/API/functionalities/limit-debug-info/onetwo.h
@@ -1,22 +1,10 @@
-#ifdef IN_ONE
-#define ONE_API LLDB_DYLIB_EXPORT
-#else
-#define ONE_API LLDB_DYLIB_IMPORT
-#endif
-
-#ifdef IN_TWO
-#define TWO_API LLDB_DYLIB_EXPORT
-#else
-#define TWO_API LLDB_DYLIB_IMPORT
-#endif
-
-struct ONE_API One {
+struct One {
   int one = 142;
   constexpr One() = default;
   virtual ~One();
 };
 
-struct TWO_API Two : One {
+struct Two : One {
   int two = 242;
   constexpr Two() = default;
   ~Two() override;



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

[Lldb-commits] [lldb] c1f1db8 - [lldb] Add a host-independent test for handling -flimit-debug-info

2020-07-02 Thread Pavel Labath via lldb-commits 

Author: Pavel Labath
Date: 2020-07-02T15:51:20+02:00
New Revision: c1f1db8502f1fc788eefeafd3dc225bd287ee4b9

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

LOG: [lldb] Add a host-independent test for handling -flimit-debug-info

This complements the existing TestLimitDebugInfo.py, which tests this
scenario more comprehensively, but is not able to run on all hosts.
Specifically, it's hard to trigger this code from windows because clang
tries hard to ensure that debug info for types marked with
__declspec(dllexport) is emitted even under -flimit-debug-info (and
dllexport is needed to use a type across shared libraries).

This assembly-based test serves two purposes:
- it tests that -flimit-debug-info code path works for windows binaries
  (even though the aforementioned feature means its less likely to be
  used there)
- it gives basic test coverage for the -flimit-debug-info handling code
  when running the test suite on windows hosts.

Added: 
lldb/test/Shell/SymbolFile/DWARF/limit-debug-info.s

Modified: 


Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/limit-debug-info.s 
b/lldb/test/Shell/SymbolFile/DWARF/limit-debug-info.s
new file mode 100644
index ..b14d6106cf06
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/limit-debug-info.s
@@ -0,0 +1,131 @@
+# REQUIRES: lld, x86
+# RUN: llvm-mc --triple=x86_64-pc-windows --filetype=obj --defsym DLL=0 %s 
>%t.dll.o
+# RUN: llvm-mc --triple=x86_64-pc-windows --filetype=obj --defsym EXE=0 %s 
>%t.exe.o
+# RUN: lld-link /OUT:%t.dll %t.dll.o /SUBSYSTEM:console /dll /noentry /debug
+# RUN: lld-link /OUT:%t.exe %t.exe.o /SUBSYSTEM:console /debug /force
+# RUN: %lldb %t.exe -o "target modules add %t.dll" -o "p var" \
+# RUN:   -o exit 2>&1 | FileCheck %s
+
+# CHECK: (lldb) p var
+# CHECK: (A) $0 = (member = 47)
+
+.section.debug_abbrev,"dr"
+.Lsection_abbrev:
+.byte   1   # Abbreviation Code
+.byte   17  # DW_TAG_compile_unit
+.byte   1   # DW_CHILDREN_yes
+.byte   37  # DW_AT_producer
+.byte   8   # DW_FORM_string
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   2   # Abbreviation Code
+.byte   52  # DW_TAG_variable
+.byte   0   # DW_CHILDREN_no
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   73  # DW_AT_type
+.byte   19  # DW_FORM_ref4
+.byte   2   # DW_AT_location
+.byte   24  # DW_FORM_exprloc
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   3   # Abbreviation Code
+.byte   19  # DW_TAG_structure_type
+.byte   1   # DW_CHILDREN_yes
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   11  # DW_AT_byte_size
+.byte   11  # DW_FORM_data1
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   4   # Abbreviation Code
+.byte   13  # DW_TAG_member
+.byte   0   # DW_CHILDREN_no
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   73  # DW_AT_type
+.byte   19  # DW_FORM_ref4
+.byte   56  # DW_AT_data_member_location
+.byte   11  # DW_FORM_data1
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   5   # Abbreviation Code
+.byte   36  # DW_TAG_base_type
+.byte   0   # DW_CHILDREN_no
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   62  # DW_AT_encoding
+.byte   11  # DW_FORM_data1
+.byte   11  # DW_AT_byte_size
+.byte   11  # DW_FORM_data1
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   6   # Abbreviation Code
+.byte   19  # DW_TAG_str

[Lldb-commits] [PATCH] D83023: [lldb/ObjectFileMachO] Fetch shared cache images from our own shared cache

2020-07-02 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

I think this is a very interesting feature (lldb being able to load modules 
from memory; the mac shared cache thingy is interesting too, but in a different 
way). We have a feature request from people who are downloading modules from a 
network (from a proprietary symbol server, etc.) and would like to pass them to 
lldb without having to serialize them to disk. This would be a step towards 
making that happen. It could also be useful for our own unit tests which now 
have to do a similar thing.

However, I think this could use some clean up. There's a lot of juggling of 
data/file/object offsets going on, and it all seems inconsistent and avoidable 
to me. Please see inline comments for details.

The patch is also quite light on testing. If done right, I believe this should 
make it possible to yaml2obj a file into memory in a unit test and then create 
a Module from that. That would enable us to test the Module(Spec) changes in 
isolation, and move them to a separate patch.

In D83023#2127232 , @friss wrote:

>   I don't suppose you care a lot about `ObjectFileMachO.cpp`


I care about ObjectFileMachO to the extent that I need to occasionally touch it 
when working on generic interfaces. And I gotta say that changing anything in 
there is pretty hard right now... :/




Comment at: lldb/include/lldb/Host/macosx/HostInfoMacOSX.h:22-43
+struct SharedCacheImageInfo {
+  lldb::offset_t unslidTextOffset;
+  UUID uuid;
+};
+
+class SharedCacheInfo {
+public:

The way we've done this elsewhere is to add the interface to the base class 
with a default stubbed-out implementation. That way, you don't have to put 
`#ifdef __APPLE__` into all of the code which tries to use this. 
`HostInfo::GetXcodeSDKPath` is the latest example of that.



Comment at: lldb/include/lldb/Host/macosx/HostInfoMacOSX.h:38
+  UUID m_uuid;
+  lldb::DataBufferSP m_data;
+

I think this could just be an ArrayRef, or a void* or something, and 
then you could create an appropriately sized DataBufferHostMemory (or whatever 
ends up called) when working with a specific module.



Comment at: lldb/include/lldb/Utility/DataBuffer.h:82
 
+class DataBufferHostMemory : public DataBuffer {
+public:

All of our other DataBuffers also point to host memory (how could they not do 
that?). I guess what really makes this one special is that it does not own the 
storage that it points to...



Comment at: lldb/source/Core/Module.cpp:154-159
+  // If the ModuleSpec provided a DataBuffer, let's respect the ModuleSpec's
+  // file offset when reading in this buffer.
+  if (data_sp) {
+file_offset = module_spec.GetObjectOffset();
+file_size = module_spec.GetData()->GetByteSize();
+  }

I think this overloads the meaning of `module_spec.GetObjectOffset()` in a 
fairly confusing way. Normally, I believe 
`ModuleSpec::GetObject{Name,Offset,Size}` is used to refer to the 
name/offse/size of a object file located in an archive (.a). However, here it 
seems you are reusing it for something else. It seems unfortunate that the 
meaning of this field should change depending on the "data" field being present.

What exactly is the purpose of this field? Could we avoid this by just creating 
an appropriately-sized DataBuffer ?



Comment at: lldb/source/Core/Module.cpp:1265-1272
+  if (m_object_size)
+file_size = m_object_size;
+  else
+file_size =
+FileSystem::Instance().GetByteSize(m_file) - m_object_offset;
+
+  if (m_data_sp)

With an appropriately sized data_sp, I'm hoping most if this could go away...



Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:935-948
+  // In the shared cache, the load command file offsets are relative to the
+  // base of the shared cache, not the dylib image.
+  Section *segment = section->GetParent().get();
+  if (!segment)
+segment = section;
+
+  // We know __TEXT is at offset 0 of the image. Compute the offset of the

Wouldn't this be better handled by adjusting the file offsets during Section 
creation?



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:42-44
+#include 
+
+#include 

Leftovers from an earlier implementation?



Comment at: lldb/source/Symbol/ObjectFile.cpp:217
+  else
+data_offset = file_offset;
+

this data/file_offset business would be nice to get rid of too...



Comment at: lldb/unittests/Host/HostInfoTest.cpp:74
+  for (const auto& image : shared_cache_info.GetImages()) {
+EXPECT_TRUE(image.getValue().unslidTextOffset < 
shared_cache_info.GetData()->GetByteSize());
+  }

EXPECT_LT



Comment at: llvm/include/llvm/BinaryFormat/MachO.h:86
+  MH_NLIST_O

[Lldb-commits] [PATCH] D81697: Add support for batch-testing to the LLDB testsuite.

2020-07-02 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

In D81697#2127061 , @bbli wrote:

> Hi, bumping my post from two weeks ago. The main question I had was: would it 
> be ok if I just brought over the Outcome object for the time being?


Umm... I don't know... Maybe? I suppose it depends on how invasive the change 
would be... If you're willing to give it a try, I am willing to look at the 
result, but I gotta warn you that I am pretty averse (and I know some of the 
other contributors are too) about modifying imported third-party code.

OTOH, if you're just looking to do something, I can think of other helpful 
fixes that would be a lot less controversial. For example, the thing that's 
annoying me now is that whenever a test fails in the "make" phase (e.g. a 
compile error), we only get an error message with the return status, and the 
original make invocation. It would be very helpful if that error could include 
the entire make output, including all the compile commands that it ran. In my 
book that would be more helpful (and a lot easier) than the batch mode.

I'd also be happy to help you with attempting to migrate to a newer unittest 
framework, if you're willing to give that a shot. Even if we don't end up doing 
it, I think just tracking down the original version this was forked from, and 
enumerating our changes to it (and their reasons) would be helpful.

In D81697#2091220 , @bbli wrote:

> - Also, `testPartExecutor` will catch all exceptions, but I believe we want 
> to exit early and propagate non `self.failureException` errors all the way up 
> for `runMethod` to handle, correct?


I don't have a strong opinion on that and I'm happy to go with whatever the 
upstream version does. In theory a different exception should not mean a 
failure, but a problem in the test itself, but in practice it usually does not 
work out that way -- a lot of failures manifest themselves `AttributeError: 
'NoneType' object has no attribute 'foo'` errors (because an object ended up 
being null when it shouldn't be) or similar. And I'm not sure that littering 
the test with self.assertNotNone(foo) is worth it...

> - Finally, by `expect_expr`, are you referring to the "expect*" methods that 
> are defined in lldbtest.py? And are you suggesting to have the error handling 
> context manager to be called inside these functions? Because wouldn't that 
> lead to the same problem of having a function continue in the situation where 
> the function has multiple `expect_expr` commands?

I was specifically referring to the `expect_expr` method, not the entire expect 
function family. You're right that this would mean that the caller of 
`expect_expr` would continue in its work, but what I wanted to do is make this 
an official part of the `expect_expr` contract. `expect_expr` is a fairly new 
function, and the way we currently use that is just to inspect values of 
various objects. There's no harm in batching those. We could just add some 
wording to its documentation to say that one should not use that function to 
mutate state as that could lead to very confusing error messages if that 
command fails.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81697



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

[Lldb-commits] [lldb] b725142 - [lldb] Fix type conversion in the Scalar getters

2020-07-02 Thread Pavel Labath via lldb-commits 

Author: Pavel Labath
Date: 2020-07-02T18:02:57+02:00
New Revision: b725142c8db8584007cb1cd9149e8bcecaa88547

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

LOG: [lldb] Fix type conversion in the Scalar getters

Summary:
The Scalar class claims to follow the C type conversion rules. This is
true for the Promote function, but it is not true for the implicit
conversions done in the getter methods.

These functions had a subtle bug: when extending the type, they used the
signedness of the *target* type in order to determine whether to do
sign-extension or zero-extension. This is not how things work in C,
which uses the signedness of the *source* type. I.e., C does
(sign-)extension before it does signed->unsigned conversion, and not the
other way around.

This means that: (unsigned long)(int)-1
  is equal to (unsigned long)0x
  and not (unsigned long)0x

Unsurprisingly, we have accumulated code which depended on this
inconsistent behavior. It mainly manifested itself as code calling
"ULongLong/SLongLong" as a way to get the value of the Scalar object in
a primitive type that is "large enough". Previously, the ULongLong
conversion did not do sign-extension, but now it does.

This patch makes the Scalar getters consistent with the declared
semantics, and fixes the couple of call sites that were using it
incorrectly.

Reviewers: teemperor, JDevlieghere

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/include/lldb/Utility/Scalar.h
lldb/source/Core/ValueObject.cpp
lldb/source/Expression/IRInterpreter.cpp
lldb/source/Utility/Scalar.cpp
lldb/test/API/commands/expression/ir-interpreter/TestIRInterpreter.py
lldb/unittests/Utility/ScalarTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Scalar.h 
b/lldb/include/lldb/Utility/Scalar.h
index 566c52c14a8d..275df4d63b0d 100644
--- a/lldb/include/lldb/Utility/Scalar.h
+++ b/lldb/include/lldb/Utility/Scalar.h
@@ -267,8 +267,7 @@ class Scalar {
   llvm::APInt m_integer;
   llvm::APFloat m_float;
 
-  template  T GetAsSigned(T fail_value) const;
-  template  T GetAsUnsigned(T fail_value) const;
+  template  T GetAs(T fail_value) const;
 
 private:
   friend const Scalar operator+(const Scalar &lhs, const Scalar &rhs);

diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index ba3d2c509be8..dfadb3c5233f 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -1203,6 +1203,7 @@ uint64_t ValueObject::GetValueAsUnsigned(uint64_t 
fail_value, bool *success) {
 if (ResolveValue(scalar)) {
   if (success)
 *success = true;
+  scalar.MakeUnsigned();
   return scalar.ULongLong(fail_value);
 }
 // fallthrough, otherwise...
@@ -1220,6 +1221,7 @@ int64_t ValueObject::GetValueAsSigned(int64_t fail_value, 
bool *success) {
 if (ResolveValue(scalar)) {
   if (success)
 *success = true;
+  scalar.MakeSigned();
   return scalar.SLongLong(fail_value);
 }
 // fallthrough, otherwise...

diff  --git a/lldb/source/Expression/IRInterpreter.cpp 
b/lldb/source/Expression/IRInterpreter.cpp
index 04d7cb3e74b6..4c7a65626598 100644
--- a/lldb/source/Expression/IRInterpreter.cpp
+++ b/lldb/source/Expression/IRInterpreter.cpp
@@ -147,7 +147,7 @@ class InterpreterStackFrame {
 return std::string(ss.GetString());
   }
 
-  bool AssignToMatchType(lldb_private::Scalar &scalar, uint64_t u64value,
+  bool AssignToMatchType(lldb_private::Scalar &scalar, llvm::APInt value,
  Type *type) {
 size_t type_size = m_target_data.getTypeStoreSize(type);
 
@@ -157,7 +157,7 @@ class InterpreterStackFrame {
 if (type_size != 1)
   type_size = PowerOf2Ceil(type_size);
 
-scalar = llvm::APInt(type_size*8, u64value);
+scalar = value.zextOrTrunc(type_size * 8);
 return true;
   }
 
@@ -171,8 +171,7 @@ class InterpreterStackFrame {
   if (!ResolveConstantValue(value_apint, constant))
 return false;
 
-  return AssignToMatchType(scalar, value_apint.getLimitedValue(),
-   value->getType());
+  return AssignToMatchType(scalar, value_apint, value->getType());
 }
 
 lldb::addr_t process_address = ResolveValue(value, module);
@@ -190,13 +189,14 @@ class InterpreterStackFrame {
 lldb::offset_t offset = 0;
 if (value_size <= 8) {
   uint64_t u64value = value_extractor.GetMaxU64(&offset, value_size);
-  return AssignToMatchType(scalar, u64value, value->getType());
+  return AssignToMatchType(scalar, llvm::APInt(64, u64value),
+   value->getType());
 }
 
 return false;
   }
 
-  bool

[Lldb-commits] [PATCH] D82772: [lldb] Fix type conversion in the Scalar getters

2020-07-02 Thread Pavel Labath via Phabricator via lldb-commits 
labath marked 3 inline comments as done.
labath added inline comments.



Comment at: lldb/source/Utility/Scalar.cpp:864
   case e_uint512:
-return static_cast(
-llvm::APIntOps::RoundAPIntToDouble(m_integer));

teemperor wrote:
> Seems unrelated to the patch? Also it would be inconsistent that this is 
> removed here and not below. FWIW this is used to suppress 
> `-Wdouble-promotion` warnings, so it does have a purpose.
Yeah, I think this ended up being copied from the function above it.



Comment at: 
lldb/test/API/commands/expression/ir-interpreter/TestIRInterpreter.py:66
+   "$i ^ $j",
+   "($ull & -1) == $u"]
 

FTR, this was working correctly before already. I'm just creating a more 
explicit test for it.



Comment at: lldb/unittests/Utility/ScalarTest.cpp:97
+TEST(ScalarTest, Getters) {
+  CheckConversion((int)0x87654321);
+  CheckConversion((unsigned int)0x87654321u);

teemperor wrote:
> If you change this to `CheckConversion(0x87654321);` then that's one 
> C-style cast less (which will make me very happy) and if someone accidentally 
> makes this a `long` literal or so we get a compiler warning (instead of the 
> compiler just silently truncating the thing).
> 
> Also I guess `short` and `char` is missing?
Changed to `(T)` to ``. `short` and `char` aren't interesting to test 
because Scalar does not have first-class support for them. They would get 
promoted to int before they reach the Scalar constructor. Long double is also 
missing -- that's because the relevant constructor is currently a booby-trap.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82772



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

[Lldb-commits] [PATCH] D82772: [lldb] Fix type conversion in the Scalar getters

2020-07-02 Thread Pavel Labath via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb725142c8db8: [lldb] Fix type conversion in the Scalar 
getters (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D82772?vs=274127&id=275143#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82772

Files:
  lldb/include/lldb/Utility/Scalar.h
  lldb/source/Core/ValueObject.cpp
  lldb/source/Expression/IRInterpreter.cpp
  lldb/source/Utility/Scalar.cpp
  lldb/test/API/commands/expression/ir-interpreter/TestIRInterpreter.py
  lldb/unittests/Utility/ScalarTest.cpp

Index: lldb/unittests/Utility/ScalarTest.cpp
===
--- lldb/unittests/Utility/ScalarTest.cpp
+++ lldb/unittests/Utility/ScalarTest.cpp
@@ -66,6 +66,44 @@
   ASSERT_TRUE(s2 >= s1);
 }
 
+template 
+static T2 ConvertHost(T1 val, T2 (Scalar::*)(T2) const) {
+  return T2(val);
+}
+
+template 
+static T2 ConvertScalar(T1 val, T2 (Scalar::*conv)(T2) const) {
+  return (Scalar(val).*conv)(T2());
+}
+
+template  static void CheckConversion(T val) {
+  SCOPED_TRACE("val = " + std::to_string(val));
+  EXPECT_EQ((signed char)val, Scalar(val).SChar());
+  EXPECT_EQ((unsigned char)val, Scalar(val).UChar());
+  EXPECT_EQ((short)val, Scalar(val).SShort());
+  EXPECT_EQ((unsigned short)val, Scalar(val).UShort());
+  EXPECT_EQ((int)val, Scalar(val).SInt());
+  EXPECT_EQ((unsigned)val, Scalar(val).UInt());
+  EXPECT_EQ((long)val, Scalar(val).SLong());
+  EXPECT_EQ((unsigned long)val, Scalar(val).ULong());
+  EXPECT_EQ((long long)val, Scalar(val).SLongLong());
+  EXPECT_EQ((unsigned long long)val, Scalar(val).ULongLong());
+  EXPECT_NEAR((float)val, Scalar(val).Float(), std::abs(val / 1e6));
+  EXPECT_NEAR((double)val, Scalar(val).Double(), std::abs(val / 1e12));
+  EXPECT_NEAR((long double)val, Scalar(val).LongDouble(), std::abs(val / 1e12));
+}
+
+TEST(ScalarTest, Getters) {
+  CheckConversion(0x87654321);
+  CheckConversion(0x87654321u);
+  CheckConversion(0x87654321l);
+  CheckConversion(0x87654321ul);
+  CheckConversion(0x8765432112345678ll);
+  CheckConversion(0x8765432112345678ull);
+  CheckConversion(42.25f);
+  CheckConversion(42.25);
+}
+
 TEST(ScalarTest, RightShiftOperator) {
   int a = 0x1000;
   int b = 0x;
@@ -336,11 +374,11 @@
 TEST(ScalarTest, TruncOrExtendTo) {
   Scalar S(0x);
   S.TruncOrExtendTo(12, true);
-  EXPECT_EQ(S.ULong(), 0xfffu);
+  EXPECT_EQ(S.UInt128(APInt()), APInt(12, 0xfffu));
   S.TruncOrExtendTo(20, true);
-  EXPECT_EQ(S.ULong(), 0xfu);
+  EXPECT_EQ(S.UInt128(APInt()), APInt(20, 0xfu));
   S.TruncOrExtendTo(24, false);
-  EXPECT_EQ(S.ULong(), 0x0fu);
+  EXPECT_EQ(S.UInt128(APInt()), APInt(24, 0x0fu));
   S.TruncOrExtendTo(16, false);
-  EXPECT_EQ(S.ULong(), 0xu);
+  EXPECT_EQ(S.UInt128(APInt()), APInt(16, 0xu));
 }
Index: lldb/test/API/commands/expression/ir-interpreter/TestIRInterpreter.py
===
--- lldb/test/API/commands/expression/ir-interpreter/TestIRInterpreter.py
+++ lldb/test/API/commands/expression/ir-interpreter/TestIRInterpreter.py
@@ -51,7 +51,8 @@
 options = lldb.SBExpressionOptions()
 options.SetLanguage(lldb.eLanguageTypeC_plus_plus)
 
-set_up_expressions = ["int $i = 9", "int $j = 3", "int $k = 5"]
+set_up_expressions = ["int $i = 9", "int $j = 3", "int $k = 5",
+"unsigned long long $ull = -1", "unsigned $u = -1"]
 
 expressions = ["$i + $j",
"$i - $j",
@@ -61,7 +62,8 @@
"$i << $j",
"$i & $j",
"$i | $j",
-   "$i ^ $j"]
+   "$i ^ $j",
+   "($ull & -1) == $u"]
 
 for expression in set_up_expressions:
 self.frame().EvaluateExpression(expression, options)
Index: lldb/source/Utility/Scalar.cpp
===
--- lldb/source/Utility/Scalar.cpp
+++ lldb/source/Utility/Scalar.cpp
@@ -644,73 +644,58 @@
   return success;
 }
 
-template  T Scalar::GetAsSigned(T fail_value) const {
-  switch (GetCategory(m_type)) {
-  case Category::Void:
-break;
-  case Category::Integral:
-return m_integer.sextOrTrunc(sizeof(T) * 8).getSExtValue();
-
-  case Category::Float: {
-llvm::APSInt result(sizeof(T) * 8, /*isUnsigned=*/false);
-bool isExact;
-m_float.convertToInteger(result, llvm::APFloat::rmTowardZero, &isExact);
-return result.getSExtValue();
-  }
-  }
-  return fail_value;
-}
-
-template  T Scalar::GetAsUnsigned(T fail_value) const {
+template  T Scalar::GetAs(T fail_value) const {
   switch (GetCategory(m_type)) {
   case Category::Void:
 break;
   case Category::Integral:
+if (IsSigned(m_type))
+  return m_integer.sextOrTrunc(sizeof(T) * 8).getSExtValue();

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-02 Thread Petr Hosek via Phabricator via lldb-commits 
phosek updated this revision to Diff 274255.

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -194,10 +194,34 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+set(llvm_system_libs ${system_libs})
+
+if(LLVM_ENABLE_ZLIB)
+  string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+  get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
+  if(NOT zlib_library)
+get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
+  endif()
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_STATIC_LIBRARY_SUFFIX AND zlib_library MATCHES ".*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_SHARED_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_SHARED_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-02 Thread Petr Hosek via Phabricator via lldb-commits 
phosek marked an inline comment as done.
phosek added inline comments.



Comment at: llvm/cmake/config-ix.cmake:514
+  if(ZLIB_FOUND)
+set(LLVM_ENABLE_ZLIB "YES" CACHE STRING
+  "Use zlib for compression/decompression if available. Can be ON, OFF, or 
FORCE_ON"

labath wrote:
> JDevlieghere wrote:
> > phosek wrote:
> > > JDevlieghere wrote:
> > > > If I understand correctly, after running the configuration phase with 
> > > > LLVM_ENABLE_ZLIB to FORCE_ON, the cached value will be overwritten by 
> > > > ON? 
> > > Correct, we used `FORCE_ON` above when invoking `find_package` setting 
> > > `REQUIRED` which makes the check fail if the library is missing. 
> > > Recording this information is important for LLVM consumers because it'll 
> > > be stored in `LLVMConfig.cmake` and AFAIU we don't want to propagate 
> > > `FORCE_ON` there.
> > I get that. My worry is that if for whatever reason the library disappears 
> > (system upgrade, package removal, ...) this will silently disable ZLIB 
> > support because now LLVM_ENABLE_ZLIB just equals on. This might sound far 
> > fetched, but happens all the time with "internal installs". This is why I 
> > prefer the ON/OFF/Auto approach because it doesn't update the cache 
> > variable, but would set the internal variable to either ON or OFF.
> I agree with Jonas, though I don't think the actual values (FORCE_ON/ON vs. 
> ON/Auto) really matter here. The important part is not overwriting the cache 
> variables specified by the user, as that can create various problems with 
> reconfigurations (like the zlib becoming unavailable example).
I've updated the change to shadow the variable as suggested by Jonas.


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

https://reviews.llvm.org/D79219



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

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-02 Thread Petr Hosek via Phabricator via lldb-commits 
phosek updated this revision to Diff 274256.

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -194,10 +194,34 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+set(llvm_system_libs ${system_libs})
+
+if(LLVM_ENABLE_ZLIB)
+  string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+  get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
+  if(NOT zlib_library)
+get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
+  endif()
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_STATIC_LIBRARY_SUFFIX AND zlib_library MATCHES ".*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_SHARED_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_SHARED_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-02 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere accepted this revision.
JDevlieghere added a comment.

Thanks Petr, LGTM


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

https://reviews.llvm.org/D79219



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

[Lldb-commits] [PATCH] D83023: [lldb/ObjectFileMachO] Fetch shared cache images from our own shared cache

2020-07-02 Thread Frederic Riss via Phabricator via lldb-commits 
friss marked 4 inline comments as done.
friss added a comment.

In D83023#2128298 , @labath wrote:

> I think this is a very interesting feature (lldb being able to load modules 
> from memory; the mac shared cache thingy is interesting too, but in a 
> different way). We have a feature request from people who are downloading 
> modules from a network (from a proprietary symbol server, etc.) and would 
> like to pass them to lldb without having to serialize them to disk. This 
> would be a step towards making that happen. It could also be useful for our 
> own unit tests which now have to do a similar thing.
>
> However, I think this could use some clean up. There's a lot of juggling of 
> data/file/object offsets going on, and it all seems inconsistent and 
> avoidable to me. Please see inline comments for details.


I'll see what can be done. My main goal while working on this was to avoid 
changing the semantics outside of the shared cache usecase. I understand fairly 
well the codepath that I added and then just moved some other bits around to 
keep the existing semantics for the rest. Happy to rework this.

> The patch is also quite light on testing. If done right, I believe this 
> should make it possible to yaml2obj a file into memory in a unit test and 
> then create a Module from that. That would enable us to test the Module(Spec) 
> changes in isolation, and move them to a separate patch.

I agree about the light testing. FWIW, this got significant living-on testing 
on Apple platforms, but some more targeted testing would be great.

> In D83023#2127232 , @friss wrote:
> 
>>   I don't suppose you care a lot about `ObjectFileMachO.cpp`
> 
> 
> I care about ObjectFileMachO to the extent that I need to occasionally touch 
> it when working on generic interfaces. And I gotta say that changing anything 
> in there is pretty hard right now... :/

Tell me about it...

I'll do a couple experiments in response to your comments. Thanks for the 
review!




Comment at: lldb/include/lldb/Host/macosx/HostInfoMacOSX.h:22-43
+struct SharedCacheImageInfo {
+  lldb::offset_t unslidTextOffset;
+  UUID uuid;
+};
+
+class SharedCacheInfo {
+public:

labath wrote:
> The way we've done this elsewhere is to add the interface to the base class 
> with a default stubbed-out implementation. That way, you don't have to put 
> `#ifdef __APPLE__` into all of the code which tries to use this. 
> `HostInfo::GetXcodeSDKPath` is the latest example of that.
Yeah. When I wrote this patch, GetXcodeSDKPath didn't exist and I did it this 
way to avoid putting a very non-generic API at the top-level. As we're fine 
with this, it's easy to change.



Comment at: lldb/include/lldb/Utility/DataBuffer.h:82
 
+class DataBufferHostMemory : public DataBuffer {
+public:

labath wrote:
> All of our other DataBuffers also point to host memory (how could they not do 
> that?). I guess what really makes this one special is that it does not own 
> the storage that it points to...
Good point. What about `DataBufferUnowned` ? (and adding a comment to explain 
what it's used for)



Comment at: lldb/source/Core/Module.cpp:154-159
+  // If the ModuleSpec provided a DataBuffer, let's respect the ModuleSpec's
+  // file offset when reading in this buffer.
+  if (data_sp) {
+file_offset = module_spec.GetObjectOffset();
+file_size = module_spec.GetData()->GetByteSize();
+  }

labath wrote:
> I think this overloads the meaning of `module_spec.GetObjectOffset()` in a 
> fairly confusing way. Normally, I believe 
> `ModuleSpec::GetObject{Name,Offset,Size}` is used to refer to the 
> name/offse/size of a object file located in an archive (.a). However, here it 
> seems you are reusing it for something else. It seems unfortunate that the 
> meaning of this field should change depending on the "data" field being 
> present.
> 
> What exactly is the purpose of this field? Could we avoid this by just 
> creating an appropriately-sized DataBuffer ?
So the shared cache is some kind of a container in some ways similar to a `.a`, 
file, in some ways very different. The main difference is that the contained 
dylibs are not each in their own subpart of the file. For example, the 
__LINKEDIT segment is shared by all of them. The load commands that describe 
the images are relative to the shared cache itself, not to one specific image.

So I reused the object_offset field in its "offset from the beginning of a 
container sense". 

I think I tried having the SharedCacheInfo return only the subpart of the cache 
that contains the image, and ran into issues with this. But I don't remember 
exactly what, and I have a much better understanding now, so I should try it 
again.



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:42-44
+#include 
+
+#include

[Lldb-commits] [PATCH] D83072: [lldb-vscode] Add Compile Unit List to Modules View

2020-07-02 Thread Yifan Shen via Phabricator via lldb-commits 
aelitashen updated this revision to Diff 275192.
aelitashen added a comment.

Remove comment line in request_getCompileUnits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83072

Files:
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1186,10 +1186,7 @@
   int num_units = curr_module.GetNumCompileUnits();
   for (int j = 0; j < num_units; j++) {
 auto curr_unit = curr_module.GetCompileUnitAtIndex(j);
-// auto unit_file_spec = curr_unit.GetFileSpec();
-// std::string unit_path = std::string(unit_file_spec.GetDirectory());
 units.emplace_back(CreateCompileUnit(curr_unit));
-// body.try_emplace(unit_file_spec.GetFilename(), unit_path);
   }
   body.try_emplace("compileUnits", std::move(units));
   break;


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1186,10 +1186,7 @@
   int num_units = curr_module.GetNumCompileUnits();
   for (int j = 0; j < num_units; j++) {
 auto curr_unit = curr_module.GetCompileUnitAtIndex(j);
-// auto unit_file_spec = curr_unit.GetFileSpec();
-// std::string unit_path = std::string(unit_file_spec.GetDirectory());
 units.emplace_back(CreateCompileUnit(curr_unit));
-// body.try_emplace(unit_file_spec.GetFilename(), unit_path);
   }
   body.try_emplace("compileUnits", std::move(units));
   break;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D83072: [lldb-vscode] Add Compile Unit List to Modules View

2020-07-02 Thread Yifan Shen via Phabricator via lldb-commits 
aelitashen created this revision.
aelitashen added reviewers: wallace, clayborg.
Herald added subscribers: lldb-commits, aprantl.
Herald added a project: LLDB.
aelitashen retitled this revision from "Add Compile Unit List to Modules View" 
to "[lldb-vscode] Add Compile Unit List to Modules View".
aelitashen updated this revision to Diff 275192.
aelitashen added a comment.

Remove comment line in request_getCompileUnits


User can expand and check compile unit list for the modules that have debug 
info.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83072

Files:
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1186,10 +1186,7 @@
   int num_units = curr_module.GetNumCompileUnits();
   for (int j = 0; j < num_units; j++) {
 auto curr_unit = curr_module.GetCompileUnitAtIndex(j);
-// auto unit_file_spec = curr_unit.GetFileSpec();
-// std::string unit_path = std::string(unit_file_spec.GetDirectory());
 units.emplace_back(CreateCompileUnit(curr_unit));
-// body.try_emplace(unit_file_spec.GetFilename(), unit_path);
   }
   body.try_emplace("compileUnits", std::move(units));
   break;


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1186,10 +1186,7 @@
   int num_units = curr_module.GetNumCompileUnits();
   for (int j = 0; j < num_units; j++) {
 auto curr_unit = curr_module.GetCompileUnitAtIndex(j);
-// auto unit_file_spec = curr_unit.GetFileSpec();
-// std::string unit_path = std::string(unit_file_spec.GetDirectory());
 units.emplace_back(CreateCompileUnit(curr_unit));
-// body.try_emplace(unit_file_spec.GetFilename(), unit_path);
   }
   body.try_emplace("compileUnits", std::move(units));
   break;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D82477: [lldb-vscode] Add Support for Module Event

2020-07-02 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

See inline comments. Changes needed are:

- test for eBroadcastBitSymbolsLoaded
- add "version" to module info
- test all data we expect in module info ('name', 'symbolFilePath', 'version', 
'symbolStatus', 'addressRange')




Comment at: lldb/test/API/tools/lldb-vscode/module/Makefile:3
+
+include Makefile.rules

We need to test eBroadcastBitSymbolsLoaded in this test. I would suggest we 
make an "a.out.stripped"  (see the makefile form 
lldb/test/API/lang/objc/objc-ivar-stripped/Makefile for how to do this).



Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:20
+def test_modules_event(self):
+program= self.getBuildArtifact("a.out")
+self.build_and_launch(program)

eBroadcastBitSymbolsLoaded needs to be tested: change this line to:
```
program_basename = 'a.out.stripped'
program = self.getBuildArtifact(program_basename)
```
after modifying the Makefile as suggested above.




Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:27
+self.continue_to_breakpoints(breakpoint_ids)
+self.assertTrue('a.out' in self.vscode.get_active_modules(), 
+'Module: a.out is loaded')

Cached active modules, and use assertIn here and switch to "a.out.stripped", 
and verify the 'name' is in module info and that path is correct:
```
active_modules = self.vscode.get_active_modules()
self.assertIn(program_basename, active_modules, '%s module is in active 
modules' % (program_basename))
program_module = active_modules[program_basename]
self.assertIn('name', program_module, 'make sure 'path' key is in module')
self.assertEqual(program, program_module['name'])
```



Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:29-30
+'Module: a.out is loaded')
+self.assertTrue('symbolFilePath' in 
self.vscode.get_active_modules()['a.out'],
+'Symbol exists')

use self.assertIn(...) and use the cached "active_modules" variable:
```
self.assertIn('symbolFilePath', program_module, 'Make sure 'symbolFilePath' key 
is in module info')
self.assertEqual(program, program_module['symbolFilePath'])
```
Then you need to verify all other information that you put into the Module like 
"symbolStatus" and "addressRange".




Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:29-30
+'Module: a.out is loaded')
+self.assertTrue('symbolFilePath' in 
self.vscode.get_active_modules()['a.out'],
+'Symbol exists')

clayborg wrote:
> use self.assertIn(...) and use the cached "active_modules" variable:
> ```
> self.assertIn('symbolFilePath', program_module, 'Make sure 'symbolFilePath' 
> key is in module info')
> self.assertEqual(program, program_module['symbolFilePath'])
> ```
> Then you need to verify all other information that you put into the Module 
> like "symbolStatus" and "addressRange".
> 
Now we would send a LLDB command line command to load the "a.out" symbols:

```
symbols = self.getBuildArtifact("a.out")
response = self.vscode.request_evaluate('`%s' % ('target symbols add -s "%s" 
"%s"' % (program, symbols)))
```
This will cause LLDB to load the symbol file "a.out" for the executable 
"a.out.stripped". This should cause the eBroadcastBitSymbolsLoaded notification 
to fire and the modules to get updated. Then you will get the active modules 
again and verify:
- program is still "program" (the full path to the a.out.stripped binary)
- 'symbolFilePath' is equal to our "symbols" variable from above (full path to 
"a.out")
- 'symbolStatus' is now set to 'Symbols Loaded.'
- 'addressRange' is still what we expect




Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:344-345
+module.GetSymbolFileSpec().GetPath(symbol_path_arr, 
sizeof(symbol_path_arr));
+std::string symbol_path(symbol_path_arr);
+object.try_emplace("symbolFilePath", symbol_path);
+  }

Do we only want to try and add this "symbolFilePath" key if the path differs 
from "module_path"?



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:351
+  return llvm::json::Value(std::move(object));
+}
+

We should also fetch the version number from the module and populate the 
"version" key. To get the version number from a module you can use:

```
uint32_t version_nums[5];
const uint32_t num_versions = module.GetVersion(version_nums, 
sizeof(version_nums));
std::string version_str;
for (uint32_t i=0; ihttps://reviews.llvm.org/D82477/new/

https://reviews.llvm.org/D82477



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

[Lldb-commits] [PATCH] D81697: Add support for batch-testing to the LLDB testsuite.

2020-07-02 Thread Benson Li via Phabricator via lldb-commits 
bbli added a comment.

> For example, the thing that's annoying me now is that whenever a test fails 
> in the "make" phase (e.g. a compile error), we only get an error message with 
> the return status, and the original make invocation.

Sure thing, how should I get started(in particular where in the codebase should 
I look for the compile aspect of this fix)?

> I'd also be happy to help you with attempting to migrate to a newer unittest 
> framework, if you're willing to give that a shot.

Hmm, how about I give it a shot after working on the above, so I get more 
background/learn more about the inner workings of unittest2 first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81697



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

[Lldb-commits] [PATCH] D82835: [lldb] Fix missing characters when autocompleting LLDB commands in REPL

2020-07-02 Thread Martin Svensson via Phabricator via lldb-commits 
poya added a comment.

Don't believe I have the necessary permissions to land this myself, so would 
need some help to get this committed to the repo. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82835



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

Re: [Lldb-commits] [PATCH] D81978: Redo of Add terminateCommands to lldb-vscode protocol

2020-07-02 Thread Florian Hahn via lldb-commits 


> On Jun 24, 2020, at 15:48, Walter  wrote:
> 
> I'll revert this patch. This seems to be the reason


Great thanks! What’s puzzling is that on some runs, it seems to find 
, but on others it does not.

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