[Lldb-commits] [PATCH] D91238: Recognize __builtin_debugtrap on arm64, advance pc past it so users can continue easily

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added reviewers: clayborg, labath.
jasonmolenda added a project: LLDB.
Herald added subscribers: omjavaid, JDevlieghere, pengfei, kristof.beyls.
jasonmolenda requested review of this revision.

This is a change I need on Darwin systems, so I'm trying to decide whether I 
put the test case in API/functionalities or in macosx, but I think it may apply 
on Linux as well.

With a __builtin_debugtrap() in a program, we want the debugger to stop 
execution there, but we want the user to get past it with a 'continue' or 
next/step.  With a __builtin_trap(), we want the debugger to stop on that 
instruction and not advance unless the user rewrites $pc manually or something.

On x86, __builtin_debugtrap() is 0xcc (the breakpoint instruction); when you 
hit that, the pc has advanced past the 0xcc.  In debugserver 
(DNBArchImplX86_64::NotifyException) when we've hit an 0xcc that was NOT a 
breakpoint debugserver inserted, it leaves the $pc past the 0xcc, so continuing 
will work.

On arm64, __builtin_debugtrap is 'brk #0xf000', this patch recognizes that 
specific instruction in DNBArchMachARM64::NotifyException and advances the pc 
past it so we get the same behavior.

The test case hits a __builtin_debugtrap(), continues past it, hits a 
__builtin_trap(), and checks that it cannot advance past that.  With this 
debugserver patch, that's how lldb behaves on both x86 darwin and arm64 darwin.

Pretty simple stuff; the only real question is whether we should make this a 
macos-only tested behavior, or include Linux as well. Anyone know how this 
works with lldb-server on linux?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91238

Files:
  lldb/test/API/functionalities/builtin-debugtrap/Makefile
  lldb/test/API/functionalities/builtin-debugtrap/TestBuiltinDebugTrap.py
  lldb/test/API/functionalities/builtin-debugtrap/main.cpp
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp

Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -524,6 +524,25 @@
 
   return true;
 }
+// detect a __builtin_debugtrap instruction pattern ("brk #0xf000")
+// and advance the $pc past it, so that the user can continue execution.
+if (exc.exc_data.size() == 2 && exc.exc_data[0] == EXC_ARM_BREAKPOINT) {
+  nub_addr_t pc = GetPC(INVALID_NUB_ADDRESS);
+  if (pc != INVALID_NUB_ADDRESS && pc > 0) {
+DNBBreakpoint *bp =
+m_thread->Process()->Breakpoints().FindByAddress(pc);
+if (bp == nullptr) {
+  uint8_t insnbuf[4];
+  if (m_thread->Process()->ReadMemory(pc, 4, insnbuf) == 4) {
+uint8_t builtin_debugtrap_insn[4] = {0x00, 0x00, 0x3e,
+ 0xd4}; // brk #0xf000
+if (memcmp(insnbuf, builtin_debugtrap_insn, 4) == 0) {
+  SetPC(pc + 4);
+}
+  }
+}
+  }
+}
 break;
   }
   return false;
Index: lldb/test/API/functionalities/builtin-debugtrap/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/builtin-debugtrap/main.cpp
@@ -0,0 +1,11 @@
+#include 
+int global = 0;
+int main()
+{
+  global = 5; // Set a breakpoint here
+  __builtin_debugtrap();
+  global = 10;
+  __builtin_trap();
+  global = 15;
+  return global;
+}
Index: lldb/test/API/functionalities/builtin-debugtrap/TestBuiltinDebugTrap.py
===
--- /dev/null
+++ lldb/test/API/functionalities/builtin-debugtrap/TestBuiltinDebugTrap.py
@@ -0,0 +1,65 @@
+"""
+Test that lldb can continue past a __builtin_debugtrap, but not a __builtin_trap
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class BuiltinDebugTrapTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+@skipIfWindows
+@skipUnlessDarwin
+
+def test(self):
+self.build()
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "// Set a breakpoint here", lldb.SBFileSpec("main.cpp"))
+
+# Continue to __builtin_debugtrap()
+process.Continue()
+if self.TraceOn():
+self.runCmd("f")
+self.runCmd("bt")
+self.runCmd("ta v global")
+
+self.assertEqual(process.GetSelectedThread().GetStopReason(), 
+ lldb.eStopReasonException)
+
+list = target.FindGlobalVariables("global", 1, lldb.eMatchTypeNormal)
+self.assertEqual(list.GetSize(), 1)
+global_value = list.GetValueAtIndex(0)
+
+self.assertEqual(global_valu

[Lldb-commits] [PATCH] D91193: [lldb] Fine tune expect() validation

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Yes, runCmd is the way to go here...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91193

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

[Lldb-commits] [PATCH] D89376: [lldb][test] Remove not_remote_testsuite_ready in favor of skipIfRemote decorator

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG66ae40ebfb83: [lldb][test] Remove not_remote_testsuite_ready 
in favor of skipIfRemote… (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/D89376/new/

https://reviews.llvm.org/D89376

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/test/API/commands/process/launch/TestProcessLaunch.py
  lldb/test/API/functionalities/load_unload/TestLoadUnload.py
  lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
  lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
  lldb/test/API/python_api/target/TestTargetAPI.py

Index: lldb/test/API/python_api/target/TestTargetAPI.py
===
--- lldb/test/API/python_api/target/TestTargetAPI.py
+++ lldb/test/API/python_api/target/TestTargetAPI.py
@@ -333,7 +333,7 @@
 self.expect(desc, exe=False,
 substrs=['Target', 'Module', 'a.out', 'Breakpoint'])
 
-@not_remote_testsuite_ready
+@skipIfRemote
 @add_test_categories(['pyapi'])
 @no_debug_info_test
 @skipIfReproducer # Inferior doesn't run during replay.
Index: lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
===
--- lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
+++ lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
@@ -14,7 +14,7 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
-@not_remote_testsuite_ready
+@skipIfRemote
 @skipUnlessWindows
 def test_cannot_save_core_unless_process_stopped(self):
 """Test that SaveCore fails if the process isn't stopped."""
@@ -28,7 +28,7 @@
 error = process.SaveCore(core)
 self.assertTrue(error.Fail())
 
-@not_remote_testsuite_ready
+@skipIfRemote
 @skipUnlessWindows
 def test_save_windows_mini_dump(self):
 """Test that we can save a Windows mini dump."""
Index: lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
===
--- lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
+++ lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
@@ -36,7 +36,7 @@
 self.hidden_dir = os.path.join(self.wd, 'hidden')
 self.hidden_lib = os.path.join(self.hidden_dir, self.lib_name)
 
-@not_remote_testsuite_ready
+@skipIfRemote
 @skipIfWindows  # Windows doesn't have dlopen and friends, dynamic libraries work differently
 @expectedFlakeyNetBSD
 @expectedFailureAll(oslist=["linux"], archs=['arm'], bugnumber="llvm.org/pr45894")
Index: lldb/test/API/functionalities/load_unload/TestLoadUnload.py
===
--- lldb/test/API/functionalities/load_unload/TestLoadUnload.py
+++ lldb/test/API/functionalities/load_unload/TestLoadUnload.py
@@ -91,7 +91,7 @@
 # libloadunload_d.so does not appear in the image list because executable
 # dependencies are resolved relative to the debuggers PWD. Bug?
 @expectedFailureAll(oslist=["freebsd", "linux", "netbsd"])
-@not_remote_testsuite_ready
+@skipIfRemote
 @skipIfWindows  # Windows doesn't have dlopen and friends, dynamic libraries work differently
 @skipIfReproducer # VFS is a snapshot.
 def test_modules_search_paths(self):
Index: lldb/test/API/commands/process/launch/TestProcessLaunch.py
===
--- lldb/test/API/commands/process/launch/TestProcessLaunch.py
+++ lldb/test/API/commands/process/launch/TestProcessLaunch.py
@@ -28,7 +28,7 @@
 self.runCmd("settings clear auto-confirm")
 TestBase.tearDown(self)
 
-@not_remote_testsuite_ready
+@skipIfRemote
 @skipIfReproducer
 def test_io(self):
 """Test that process launch I/O redirection flags work properly."""
@@ -82,7 +82,7 @@
 # rdar://problem/9056462
 # The process launch flag '-w' for setting the current working directory
 # not working?
-@not_remote_testsuite_ready
+@skipIfRemote
 @expectedFailureAll(oslist=["freebsd", "linux"], bugnumber="llvm.org/pr20265")
 @expectedFailureNetBSD
 @skipIfReproducer
@@ -112,7 +112,7 @@
 "error:.* No such file or directory: %s" %
 invalid_dir_path])
 
-@not_remote_testsuite_ready
+@skipIfRemote
 @skipIfReproducer
 def test_set_working_dir_existing(self):
 """Test that '-w dir' sets the working dir when running the inferior."""
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lld

[Lldb-commits] [lldb] 66ae40e - [lldb][test] Remove not_remote_testsuite_ready in favor of skipIfRemote decorator

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-11-11T09:14:54+01:00
New Revision: 66ae40ebfb83c8beb2080123d3866bd08520fcef

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

LOG: [lldb][test] Remove not_remote_testsuite_ready in favor of skipIfRemote 
decorator

Those two decorators have identical behaviour. This removes
`not_remote_testsuite_ready` as `skipIfRemote` seems more consistent with the
other decorator names we have

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/test/API/commands/process/launch/TestProcessLaunch.py
lldb/test/API/functionalities/load_unload/TestLoadUnload.py
lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
lldb/test/API/python_api/target/TestTargetAPI.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index 4dfc7b108ff3..07b1d15810ab 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -390,13 +390,6 @@ def should_skip_llgs_tests():
 return skipTestIfFn(should_skip_llgs_tests)(func)
 
 
-def not_remote_testsuite_ready(func):
-"""Decorate the item as a test which is not ready yet for remote 
testsuite."""
-def is_remote():
-return "Not ready for remote testsuite" if lldb.remote_platform else 
None
-return skipTestIfFn(is_remote)(func)
-
-
 def expectedFailureOS(
 oslist,
 bugnumber=None,

diff  --git a/lldb/test/API/commands/process/launch/TestProcessLaunch.py 
b/lldb/test/API/commands/process/launch/TestProcessLaunch.py
index 83bc25d3c9f9..7a1a9a146999 100644
--- a/lldb/test/API/commands/process/launch/TestProcessLaunch.py
+++ b/lldb/test/API/commands/process/launch/TestProcessLaunch.py
@@ -28,7 +28,7 @@ def tearDown(self):
 self.runCmd("settings clear auto-confirm")
 TestBase.tearDown(self)
 
-@not_remote_testsuite_ready
+@skipIfRemote
 @skipIfReproducer
 def test_io(self):
 """Test that process launch I/O redirection flags work properly."""
@@ -82,7 +82,7 @@ def test_io(self):
 # rdar://problem/9056462
 # The process launch flag '-w' for setting the current working directory
 # not working?
-@not_remote_testsuite_ready
+@skipIfRemote
 @expectedFailureAll(oslist=["freebsd", "linux"], 
bugnumber="llvm.org/pr20265")
 @expectedFailureNetBSD
 @skipIfReproducer
@@ -112,7 +112,7 @@ def test_set_working_dir_nonexisting(self):
 "error:.* No such file or directory: %s" %
 invalid_dir_path])
 
-@not_remote_testsuite_ready
+@skipIfRemote
 @skipIfReproducer
 def test_set_working_dir_existing(self):
 """Test that '-w dir' sets the working dir when running the 
inferior."""

diff  --git a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py 
b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
index 05dffc0df734..913265549c81 100644
--- a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
+++ b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
@@ -91,7 +91,7 @@ def setSvr4Support(self, enabled):
 # libloadunload_d.so does not appear in the image list because executable
 # dependencies are resolved relative to the debuggers PWD. Bug?
 @expectedFailureAll(oslist=["freebsd", "linux", "netbsd"])
-@not_remote_testsuite_ready
+@skipIfRemote
 @skipIfWindows  # Windows doesn't have dlopen and friends, dynamic 
libraries work 
diff erently
 @skipIfReproducer # VFS is a snapshot.
 def test_modules_search_paths(self):

diff  --git 
a/lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py 
b/lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
index 0ec117222b95..b54435e7338e 100644
--- a/lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
+++ b/lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
@@ -36,7 +36,7 @@ def setUp(self):
 self.hidden_dir = os.path.join(self.wd, 'hidden')
 self.hidden_lib = os.path.join(self.hidden_dir, self.lib_name)
 
-@not_remote_testsuite_ready
+@skipIfRemote
 @skipIfWindows  # Windows doesn't have dlopen and friends, dynamic 
libraries work 
diff erently
 @expectedFlakeyNetBSD
 @expectedFailureAll(oslist=["linux"], archs=['arm'], 
bugnumber="llvm.org/pr45894")

diff  --git 
a/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py 
b/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
index d561af02d307..3812b197a32e 100644
--

[Lldb-commits] [PATCH] D91216: [lldb] [Process/FreeBSDRemote] Access GPR via reginfo offsets

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Regarding the asserts, I'm thinking if it wouldn't be better to check the 
compare the resulting RegisterInfo structs instead of the raw struct layout. 
The reasons for that are:

- it will be checking the thing that we're actually relying on (the register 
info macros is pretty complex and can be messed up easily
- the register info artifacts (unlike the GPR structs) are exported from these 
files, so the checks for that could be placed into a unit test

What do you think?


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

https://reviews.llvm.org/D91216

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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Fix the C test


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

https://reviews.llvm.org/D77153

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
  
lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  lldb/test/API/lang/c/anonymous/TestAnonymous.py

Index: lldb/test/API/lang/c/anonymous/TestAnonymous.py
===
--- lldb/test/API/lang/c/anonymous/TestAnonymous.py
+++ lldb/test/API/lang/c/anonymous/TestAnonymous.py
@@ -62,7 +62,7 @@
 
 # These should display correctly.
 self.expect("expression pz", VARIABLES_DISPLAYED_CORRECTLY,
-substrs=["(type_z *) $", " = 0x"])
+substrs=["(type_z *) $", " = 0x0"])
 
 self.expect("expression z.y", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["(type_y) $", "dummy = 2"])
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -74,6 +74,7 @@
 std::u32string u32_string(U"🍄🍅🍆🍌");
 std::u32string u32_empty(U"");
 std::basic_string uchar(5, 'a');
+std::string *null_str = nullptr;
 
 #if _LIBCPP_ABI_VERSION == 1
 std::string garbage1, garbage2, garbage3, garbage4, garbage5;
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -77,6 +77,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 self.runCmd("n")
@@ -114,6 +115,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 # The test assumes that std::string is in its cap-size-data layout.
Index: lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
@@ -39,4 +39,4 @@
 # Both `std::vector` and the type of the member have forward
 # declarations before their definitions.
 self.expect("expr --raw -- v",
-substrs=['(std::__1::vector) $0 = {', 'f = 0x', '}'])
+substrs=['(std::__1::vector) $0 = {', 'f = nullptr', '}'])
Index: lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
@@ -27,6 +27,8 @@
 return lldb::eLanguageTypeObjC_plus_plus;
   }
 
+  llvm::StringRef NilReferenceSummaryString() override { return "nil"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() const override { return &m_highlighter; }
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -119,6 +119,8 @@
 
   bool IsNilReference(ValueObject &valobj) override;
 
+  llvm::StringRef NilReferenceSummaryString() override { return "nil"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() c

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/DataFormatters/ValueObjectPrinter.cpp:367
+  else
+summary.assign("0x0");
+} else if (IsUninitialized())

If we had a C runtime we could print "NULL", but I don't think it's worth 
adding that just for this. Another alternative would be to just have a switch 
based on the `LanguageType`, but that seems like a pretty bad idea from a 
layering perspective. 


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

https://reviews.llvm.org/D77153

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

[Lldb-commits] [PATCH] D91216: [lldb] [Process/FreeBSDRemote] Access GPR via reginfo offsets

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

mgorny added a comment.

In D91216#2387909 , @labath wrote:

> Regarding the asserts, I'm thinking if it wouldn't be better to check the 
> compare the resulting RegisterInfo structs instead of the raw struct layout. 
> The reasons for that are:
>
> - it will be checking the thing that we're actually relying on (the register 
> info macros is pretty complex and can be messed up easily
> - the register info artifacts (unlike the GPR structs) are exported from 
> these files, so the checks for that could be placed into a unit test
>
> What do you think?

I presume you mean verifying that `byte_offset`s and `byte_size`s are written 
correctly to the struct? I suppose that makes sense.

I like the idea that static asserts are going to blow up during compilation 
already but I suppose it's not very important as I don't expect these 
structures to change.


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

https://reviews.llvm.org/D91216

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

[Lldb-commits] [PATCH] D91238: Recognize __builtin_debugtrap on arm64, advance pc past it so users can continue easily

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: jingham.
labath added a comment.

Including Jim, as he was involved in the discussion last time this came around 
http://lists.llvm.org/pipermail/lldb-dev/2020-March/016017.html.

This functionality would not work on linux/lldb-server currently, though I 
would love if it did -- I've found the x86 behavior of continuing after an int3 
to be pretty handy, and was annoyed by the fact it does not work the same way 
on arm.

That said, I'm not sure that implementing this in the server is the right 
approach. The previous discussion seemed to converge on doing that in the 
client, and I think the reasoning behind that is sound (like, ensuring that the 
PC we report will be the same as the one extracted by the native 
crash/backtrace tools when the process when the process hits that instruction 
without the debugger present). It would also mean that this behavior would 
automatically work with all stubs, even third party ones we have no control 
over.

Maybe we don't even have to do the really fancy thing of adjusting the PC just 
before we continue. The initial implementation could just adjust the PC 
immediately after a pc. That way, if this will be a client feature, we could 
easily go back and implement something more fancy...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91238

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

[Lldb-commits] [PATCH] D91216: [lldb] [Process/FreeBSDRemote] Access GPR via reginfo offsets

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D91216#2387956 , @mgorny wrote:

> In D91216#2387909 , @labath wrote:
>
>> Regarding the asserts, I'm thinking if it wouldn't be better to check the 
>> compare the resulting RegisterInfo structs instead of the raw struct layout. 
>> The reasons for that are:
>>
>> - it will be checking the thing that we're actually relying on (the register 
>> info macros is pretty complex and can be messed up easily
>> - the register info artifacts (unlike the GPR structs) are exported from 
>> these files, so the checks for that could be placed into a unit test
>>
>> What do you think?
>
> I presume you mean verifying that `byte_offset`s and `byte_size`s are written 
> correctly to the struct?

Yes, exactly.

> I like the idea that static asserts are going to blow up during compilation 
> already but I suppose it's not very important as I don't expect these 
> structures to change.

That's true, but I've found that the inclusion of system headers (with all 
their acoompanying ifdefs) looks pretty bad...


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

https://reviews.llvm.org/D91216

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

[Lldb-commits] [PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting

[Kadir Cetinkaya via Phabricator via lldb-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:636
+// Filter out any reformat fixits, we don't handle these.
+// FIXME: Can we?
+llvm::erase_if(FixIts,

in theory yes, as we have access to source manager, we can fetch file contents 
and create formatted  replacements (see `cleanupAndFormat`). but formatting 
those fixes can imply significant delays on clangd's diagnostic cycles (if 
there are many of those), that's the reason why we currently don't format 
fixits.



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:637
+// FIXME: Can we?
+llvm::erase_if(FixIts,
+   [](const FixItHint &Fix) { return Fix.isReformatFixit(); });

rather than doing an extra loop, can we just skip those in the for loop below ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91103

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

[Lldb-commits] [PATCH] D90490: [intel-pt][trace] Implement a "get supported trace type" packet

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

I didn't check all the details, but seems ok to me.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:3465-3468
+  if (SendPacketAndWaitForResponse(escaped_packet.GetString(), response,
+   true) ==
+  GDBRemoteCommunication::PacketResult::Success) {
+if (response.IsNormalResponse()) {

Use early exits to avoid nesting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90490

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

[Lldb-commits] [PATCH] D87442: [lldb] Show flags for memory regions

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think this looks pretty good now. Some questions inline...




Comment at: lldb/source/Target/MemoryRegionInfo.cpp:10
 #include "lldb/Target/MemoryRegionInfo.h"
+#include "lldb/Utility/LLDBAssert.h"
 

unused?



Comment at: lldb/test/API/linux/aarch64/mte_memory_region/main.c:9-14
+  if (!(getauxval(AT_HWCAP2) & HWCAP2_MTE))
+return 1;
+
+  int got = prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0);
+  if (got)
+return 1;

Instead of duplicating these checks in dotest, maybe we could use the result of 
the inferior as a indication to skip the test. Like, if, instead of hitting the 
breakpoint, the inferior exits with code 47, we know that the required cpu 
feature is not supported?



Comment at: lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp:46-48
+  llvm::handleAllErrors(
+  Info.takeError(),
+  [this](const llvm::StringError &e) { err_str = e.getMessage(); });

err_str = toString(Info.takeError());


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87442

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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

[Raphael Isemann via Phabricator via lldb-commits]

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

A few comments, but otherwise this seems good.




Comment at: lldb/include/lldb/Target/Language.h:214
 
+  virtual llvm::StringRef NilReferenceSummaryString() { return {}; }
+

`/// Returns the summary string that should be used for a ValueObject for which 
IsNilReference() is true` or something like that.

Also I think this probably should have a `Get` prefix like the other methods in 
this class.



Comment at: lldb/source/DataFormatters/ValueObjectPrinter.cpp:367
+  else
+summary.assign("0x0");
+} else if (IsUninitialized())

JDevlieghere wrote:
> If we had a C runtime we could print "NULL", but I don't think it's worth 
> adding that just for this. Another alternative would be to just have a switch 
> based on the `LanguageType`, but that seems like a pretty bad idea from a 
> layering perspective. 
Not sure what to think about hardcoding `0x0` here. You can specify all kind of 
formatters that would format the value here differently and this seems kinda 
inconsistent.
```
(lldb) expr --format d -- nullptr
(nullptr_t) $2 = 0
(lldb) expr --format b -- nullptr
(nullptr_t) $3 = 
0b
```

Could we just fall to the default formatter if we don't have a special summary 
from the plugin?



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:1138
+bool CPlusPlusLanguage::IsNilReference(ValueObject &valobj) {
+  if (valobj.GetCompilerType().GetMinimumLanguage() !=
+  eLanguageTypeC_plus_plus ||

Nit: 
`!Language::LanguageIsCPlusPlus(valobj.GetCompilerType().GetMinimumLanguage())` 
? (as we might return a specific C++ standard in the future from 
GetMinimumLanguage).


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

https://reviews.llvm.org/D77153

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

[Lldb-commits] [PATCH] D91206: [lldb] Switch expect to runCmd in TestRecursiveTypes (NFC)

[Raphael Isemann via Phabricator via lldb-commits]

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

I don't think we should re-add the bogus `RUN_SUCCEEDED` messages here. 
`self.runCmd("print tpi")` is undocumented, but still better than wrong 
documentation. Beside that this LGTM, thanks!




Comment at: lldb/test/API/types/TestRecursiveTypes.py:53
 
-self.expect("print tpi")
-self.expect("print *tpi")
+self.runCmd("print tpi", RUN_SUCCEEDED)
+self.runCmd("print *tpi", RUN_SUCCEEDED)





Comment at: lldb/test/API/types/TestRecursiveTypes.py:54
+self.runCmd("print tpi", RUN_SUCCEEDED)
+self.runCmd("print *tpi", RUN_SUCCEEDED)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91206

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

[Lldb-commits] [PATCH] D91216: [lldb] [Process/FreeBSDRemote] Access GPR via reginfo offsets

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

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

Updated to use unittests instead of static asserts.


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

https://reviews.llvm.org/D91216

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
  lldb/unittests/Process/CMakeLists.txt
  lldb/unittests/Process/FreeBSD/CMakeLists.txt
  lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp

Index: lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp
===
--- /dev/null
+++ lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp
@@ -0,0 +1,98 @@
+//===-- RegisterContextFreeBSDTests.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
+//
+//===--===//
+
+// clang-format off
+#include 
+#include 
+// clang-format on
+
+#include "gtest/gtest.h"
+
+#if defined(__x86_64__) || defined(__i386__)
+#include "lldb-x86-register-enums.h"
+#include "RegisterContextFreeBSD_i386.h"
+#endif
+#if defined(__x86_64__)
+#include "RegisterContextFreeBSD_x86_64.h"
+#endif
+
+using namespace lldb_private;
+
+#if defined(__x86_64__)
+#define ASSERT_GPR_X86_64(regname)\
+  { \
+const RegisterInfo *reg_info = ®_ctx.GetRegisterInfo()[lldb_##regname##_x86_64]; \
+EXPECT_EQ(reg_info->byte_offset, offsetof(reg, r_##regname)); \
+EXPECT_EQ(reg_info->byte_size, sizeof(reg::r_##regname)); \
+  }
+
+TEST(RegisterContextFreeBSDTest, x86_64) {
+  ArchSpec arch{"x86_64-unknown-freebsd12.2"};
+  RegisterContextFreeBSD_x86_64 reg_ctx{arch};
+
+  ASSERT_GPR_X86_64(r15);
+  ASSERT_GPR_X86_64(r14);
+  ASSERT_GPR_X86_64(r13);
+  ASSERT_GPR_X86_64(r12);
+  ASSERT_GPR_X86_64(r11);
+  ASSERT_GPR_X86_64(r10);
+  ASSERT_GPR_X86_64(r9);
+  ASSERT_GPR_X86_64(r8);
+  ASSERT_GPR_X86_64(rdi);
+  ASSERT_GPR_X86_64(rsi);
+  ASSERT_GPR_X86_64(rbp);
+  ASSERT_GPR_X86_64(rbx);
+  ASSERT_GPR_X86_64(rdx);
+  ASSERT_GPR_X86_64(rcx);
+  ASSERT_GPR_X86_64(rax);
+  ASSERT_GPR_X86_64(fs);
+  ASSERT_GPR_X86_64(gs);
+  ASSERT_GPR_X86_64(es);
+  ASSERT_GPR_X86_64(ds);
+  ASSERT_GPR_X86_64(rip);
+  ASSERT_GPR_X86_64(cs);
+  ASSERT_GPR_X86_64(rflags);
+  ASSERT_GPR_X86_64(rsp);
+  ASSERT_GPR_X86_64(ss);
+}
+#endif
+
+#if defined(__i386__) || defined(__x86_64__)
+#if defined(__i386__)
+#define reg32 reg
+#endif
+
+#define ASSERT_GPR_I386(regname)\
+  { \
+const RegisterInfo *reg_info = ®_ctx.GetRegisterInfo()[lldb_##regname##_i386]; \
+EXPECT_EQ(reg_info->byte_offset, offsetof(reg32, r_##regname)); \
+EXPECT_EQ(reg_info->byte_size, sizeof(reg32::r_##regname)); \
+  }
+
+TEST(RegisterContextFreeBSDTest, i386) {
+  ArchSpec arch{"i686-unknown-freebsd12.2"};
+  RegisterContextFreeBSD_i386 reg_ctx{arch};
+
+  ASSERT_GPR_I386(fs);
+  ASSERT_GPR_I386(es);
+  ASSERT_GPR_I386(ds);
+  ASSERT_GPR_I386(edi);
+  ASSERT_GPR_I386(esi);
+  ASSERT_GPR_I386(ebp);
+  ASSERT_GPR_I386(ebx);
+  ASSERT_GPR_I386(edx);
+  ASSERT_GPR_I386(ecx);
+  ASSERT_GPR_I386(eax);
+  ASSERT_GPR_I386(eip);
+  ASSERT_GPR_I386(cs);
+  ASSERT_GPR_I386(eflags);
+  ASSERT_GPR_I386(esp);
+  ASSERT_GPR_I386(ss);
+  ASSERT_GPR_I386(gs);
+}
+#endif
Index: lldb/unittests/Process/FreeBSD/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Process/FreeBSD/CMakeLists.txt
@@ -0,0 +1,9 @@
+add_lldb_unittest(RegisterContextFreeBSDTests
+  RegisterContextFreeBSDTests.cpp
+
+  LINK_LIBS
+lldbPluginProcessUtility
+  )
+
+target_include_directories(RegisterContextFreeBSDTests PRIVATE
+  ${LLDB_SOURCE_DIR}/source/Plugins/Process/Utility)
Index: lldb/unittests/Process/CMakeLists.txt
===
--- lldb/unittests/Process/CMakeLists.txt
+++ lldb/unittests/Process/CMakeLists.txt
@@ -1,4 +1,7 @@
 add_subdirectory(gdb-remote)
+if (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
+  add_subdirectory(FreeBSD)
+endif()
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
   add_subdirectory(Linux)
   add_subdirectory(POSIX)
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
@@ -18,6 +18,8 @@
 #include 
 // clang-format on
 
+#include 
+
 #include "Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.

[Lldb-commits] [PATCH] D91241: [LLDB] Make offset field optional in RegisterInfo packet for Arm64

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid created this revision.
omjavaid added a reviewer: labath.
Herald added a subscriber: kristof.beyls.
omjavaid requested review of this revision.

This patch carries forward our aim to remove offset field from qRegisterInfo 
packets and XML register description. I have created a new function which 
returns if offset fields are dynamic meaning client can calculate offset on its 
own based on register number sequence and register size. For now this function 
only returns true for NativeRegisterContextLinux_arm64 but we can test this for 
other architectures and make it standard later.

As a consequence we do not send offset field from lldb-server (arm64 for now) 
while other stubs dont have an offset field so it wont effect them for now. On 
the client side we already have a mechanism to calculate the offset but a minor 
adjustment has been made to make offset increment only for primary registers.

Also some tests have been adjusted for this change.


https://reviews.llvm.org/D91241

Files:
  lldb/include/lldb/Host/common/NativeRegisterContext.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  
lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
  lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
  lldb/unittests/tools/lldb-server/tests/TestClient.cpp

Index: lldb/unittests/tools/lldb-server/tests/TestClient.cpp
===
--- lldb/unittests/tools/lldb-server/tests/TestClient.cpp
+++ lldb/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -219,6 +219,7 @@
 }
 
 Error TestClient::qRegisterInfos() {
+  uint32_t reg_offset = 0;
   for (unsigned int Reg = 0;; ++Reg) {
 std::string Message = formatv("qRegisterInfo{0:x-}", Reg).str();
 Expected InfoOr = SendMessage(Message);
@@ -227,6 +228,12 @@
   break;
 }
 m_register_infos.emplace_back(std::move(*InfoOr));
+
+if (m_register_infos[Reg].byte_offset == LLDB_INVALID_INDEX32)
+  m_register_infos[Reg].byte_offset = reg_offset;
+
+reg_offset =
+m_register_infos[Reg].byte_offset + m_register_infos[Reg].byte_size;
 if (m_register_infos[Reg].kinds[eRegisterKindGeneric] ==
 LLDB_REGNUM_GENERIC_PC)
   m_pc_register = Reg;
Index: lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
===
--- lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
+++ lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
@@ -171,7 +171,7 @@
   Info.byte_size /= CHAR_BIT;
 
   if (!to_integer(Elements["offset"], Info.byte_offset, 10))
-return make_parsing_error("qRegisterInfo: offset");
+Info.byte_offset = LLDB_INVALID_INDEX32;
 
   Info.encoding = Args::StringToEncoding(Elements["encoding"]);
   if (Info.encoding == eEncodingInvalid)
Index: lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
===
--- lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
+++ lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
@@ -65,5 +65,8 @@
 self.assertEqual(q_info_reg["set"], xml_info_reg.get("group"))
 self.assertEqual(q_info_reg["format"], xml_info_reg.get("format"))
 self.assertEqual(q_info_reg["bitsize"], xml_info_reg.get("bitsize"))
-self.assertEqual(q_info_reg["offset"], xml_info_reg.get("offset"))
+
+if not self.getArchitecture() == 'aarch64':
+self.assertEqual(q_info_reg["offset"], xml_info_reg.get("offset"))
+
 self.assertEqual(q_info_reg["encoding"], xml_info_reg.get("encoding"))
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -563,11 +563,13 @@
 
 reg_info.byte_offset = reg_offset;
 assert(reg_info.byte_size != 0);
-reg_offset += reg_info.byte_size;
+
 if (!value_regs.empty()) {
   value_regs.push_back(LLDB_INVALID_REGNUM);
   reg_info.value_regs = value_regs.data();
-}
+} else
+  reg_offset += reg_info.byte_size;
+
 if (!invalidate_regs.empty()) {
   invalidate_regs.push_back(LLDB_INVALID_REGNUM);
   reg_info.invalidate_regs = invalidate_regs.data();
@@ -4428,11 +4430,13 @@
 
 reg_info.byte_offset = reg_offset;
 assert(reg_info.byte_size != 0);
-reg_o

[Lldb-commits] [PATCH] D87442: [lldb] Show flags for memory regions

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/test/API/linux/aarch64/mte_memory_region/main.c:9-14
+  if (!(getauxval(AT_HWCAP2) & HWCAP2_MTE))
+return 1;
+
+  int got = prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0);
+  if (got)
+return 1;

labath wrote:
> Instead of duplicating these checks in dotest, maybe we could use the result 
> of the inferior as a indication to skip the test. Like, if, instead of 
> hitting the breakpoint, the inferior exits with code 47, we know that the 
> required cpu feature is not supported?
Sounds good to me. 

That would mean defining things like PROT_MTE in the test file, for toolchains 
that won't have it. I assume that's ok to do.
(I'll probably need to do that for lldb-server code later anyway)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87442

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

[Lldb-commits] [PATCH] D87442: [lldb] Show flags for memory regions

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 304441.
DavidSpickett added a comment.

- Remove unused assert header
- Simplify error handling in LinuxProcMapsTest


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87442

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/docs/use/qemu-testing.rst
  lldb/include/lldb/Target/MemoryRegionInfo.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/scripts/lldb-test-qemu/run-qemu.sh
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp
  lldb/source/Plugins/Process/Utility/LinuxProcMaps.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
  lldb/source/Target/MemoryRegionInfo.cpp
  lldb/test/API/linux/aarch64/mte_memory_region/Makefile
  
lldb/test/API/linux/aarch64/mte_memory_region/TestAArch64LinuxMTEMemoryRegion.py
  lldb/test/API/linux/aarch64/mte_memory_region/main.c
  lldb/unittests/Process/CMakeLists.txt
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
  lldb/unittests/Process/minidump/MinidumpParserTest.cpp

Index: lldb/unittests/Process/minidump/MinidumpParserTest.cpp
===
--- lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -378,15 +378,15 @@
   parser->BuildMemoryRegions(),
   testing::Pair(testing::ElementsAre(
 MemoryRegionInfo({0x0, 0x1}, no, no, no, no,
- ConstString(), unknown, 0),
+ ConstString(), unknown, 0, unknown),
 MemoryRegionInfo({0x1, 0x21000}, yes, yes, no, yes,
- ConstString(), unknown, 0),
+ ConstString(), unknown, 0, unknown),
 MemoryRegionInfo({0x4, 0x1000}, yes, no, no, yes,
- ConstString(), unknown, 0),
+ ConstString(), unknown, 0, unknown),
 MemoryRegionInfo({0x7ffe, 0x1000}, yes, no, no, yes,
- ConstString(), unknown, 0),
+ ConstString(), unknown, 0, unknown),
 MemoryRegionInfo({0x7ffe1000, 0xf000}, no, no, no, yes,
- ConstString(), unknown, 0)),
+ ConstString(), unknown, 0, unknown)),
 true));
 }
 
@@ -409,12 +409,13 @@
 
   EXPECT_THAT(
   parser->BuildMemoryRegions(),
-  testing::Pair(testing::ElementsAre(
-MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown,
- yes, ConstString(), unknown, 0),
-MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown,
- yes, ConstString(), unknown, 0)),
-false));
+  testing::Pair(
+  testing::ElementsAre(
+  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, yes,
+   ConstString(), unknown, 0, unknown),
+  MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, yes,
+   ConstString(), unknown, 0, unknown)),
+  false));
 }
 
 TEST_F(MinidumpParserTest, GetMemoryRegionInfoFromMemory64List) {
@@ -424,12 +425,13 @@
   // we don't have a MemoryInfoListStream.
   EXPECT_THAT(
   parser->BuildMemoryRegions(),
-  testing::Pair(testing::ElementsAre(
-MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown,
- yes, ConstString(), unknown, 0),
-MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown,
- yes, ConstString(), unknown, 0)),
-false));
+  testing::Pair(
+  testing::ElementsAre(
+  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, yes,
+   ConstString(), unknown, 0, unknown),
+  MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, yes,
+   ConstString(), unknown, 0, unknown)),
+  false));
 }
 
 TEST_F(MinidumpParserTest, GetMemoryRegionInfoLinuxMaps) {
@@ -453,22 +455,42 @@
   ConstString app_process("/system/bin/app_process");
   ConstString linker("/system

[Lldb-commits] [PATCH] D91216: [lldb] [Process/FreeBSDRemote] Access GPR via reginfo offsets

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

mgorny updated this revision to Diff 304445.
mgorny added a comment.

clang-format


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

https://reviews.llvm.org/D91216

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
  lldb/unittests/Process/CMakeLists.txt
  lldb/unittests/Process/FreeBSD/CMakeLists.txt
  lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp

Index: lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp
===
--- /dev/null
+++ lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp
@@ -0,0 +1,100 @@
+//===-- RegisterContextFreeBSDTests.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
+//
+//===--===//
+
+// clang-format off
+#include 
+#include 
+// clang-format on
+
+#include "gtest/gtest.h"
+
+#if defined(__x86_64__) || defined(__i386__)
+#include "lldb-x86-register-enums.h"
+#include "RegisterContextFreeBSD_i386.h"
+#endif
+#if defined(__x86_64__)
+#include "RegisterContextFreeBSD_x86_64.h"
+#endif
+
+using namespace lldb_private;
+
+#if defined(__x86_64__)
+#define ASSERT_GPR_X86_64(regname) \
+  {\
+const RegisterInfo *reg_info = \
+®_ctx.GetRegisterInfo()[lldb_##regname##_x86_64];   \
+EXPECT_EQ(reg_info->byte_offset, offsetof(reg, r_##regname));  \
+EXPECT_EQ(reg_info->byte_size, sizeof(reg::r_##regname));  \
+  }
+
+TEST(RegisterContextFreeBSDTest, x86_64) {
+  ArchSpec arch{"x86_64-unknown-freebsd12.2"};
+  RegisterContextFreeBSD_x86_64 reg_ctx{arch};
+
+  ASSERT_GPR_X86_64(r15);
+  ASSERT_GPR_X86_64(r14);
+  ASSERT_GPR_X86_64(r13);
+  ASSERT_GPR_X86_64(r12);
+  ASSERT_GPR_X86_64(r11);
+  ASSERT_GPR_X86_64(r10);
+  ASSERT_GPR_X86_64(r9);
+  ASSERT_GPR_X86_64(r8);
+  ASSERT_GPR_X86_64(rdi);
+  ASSERT_GPR_X86_64(rsi);
+  ASSERT_GPR_X86_64(rbp);
+  ASSERT_GPR_X86_64(rbx);
+  ASSERT_GPR_X86_64(rdx);
+  ASSERT_GPR_X86_64(rcx);
+  ASSERT_GPR_X86_64(rax);
+  ASSERT_GPR_X86_64(fs);
+  ASSERT_GPR_X86_64(gs);
+  ASSERT_GPR_X86_64(es);
+  ASSERT_GPR_X86_64(ds);
+  ASSERT_GPR_X86_64(rip);
+  ASSERT_GPR_X86_64(cs);
+  ASSERT_GPR_X86_64(rflags);
+  ASSERT_GPR_X86_64(rsp);
+  ASSERT_GPR_X86_64(ss);
+}
+#endif
+
+#if defined(__i386__) || defined(__x86_64__)
+#if defined(__i386__)
+#define reg32 reg
+#endif
+
+#define ASSERT_GPR_I386(regname)   \
+  {\
+const RegisterInfo *reg_info = \
+®_ctx.GetRegisterInfo()[lldb_##regname##_i386]; \
+EXPECT_EQ(reg_info->byte_offset, offsetof(reg32, r_##regname));\
+EXPECT_EQ(reg_info->byte_size, sizeof(reg32::r_##regname));\
+  }
+
+TEST(RegisterContextFreeBSDTest, i386) {
+  ArchSpec arch{"i686-unknown-freebsd12.2"};
+  RegisterContextFreeBSD_i386 reg_ctx{arch};
+
+  ASSERT_GPR_I386(fs);
+  ASSERT_GPR_I386(es);
+  ASSERT_GPR_I386(ds);
+  ASSERT_GPR_I386(edi);
+  ASSERT_GPR_I386(esi);
+  ASSERT_GPR_I386(ebp);
+  ASSERT_GPR_I386(ebx);
+  ASSERT_GPR_I386(edx);
+  ASSERT_GPR_I386(ecx);
+  ASSERT_GPR_I386(eax);
+  ASSERT_GPR_I386(eip);
+  ASSERT_GPR_I386(cs);
+  ASSERT_GPR_I386(eflags);
+  ASSERT_GPR_I386(esp);
+  ASSERT_GPR_I386(ss);
+  ASSERT_GPR_I386(gs);
+}
+#endif
Index: lldb/unittests/Process/FreeBSD/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Process/FreeBSD/CMakeLists.txt
@@ -0,0 +1,9 @@
+add_lldb_unittest(RegisterContextFreeBSDTests
+  RegisterContextFreeBSDTests.cpp
+
+  LINK_LIBS
+lldbPluginProcessUtility
+  )
+
+target_include_directories(RegisterContextFreeBSDTests PRIVATE
+  ${LLDB_SOURCE_DIR}/source/Plugins/Process/Utility)
Index: lldb/unittests/Process/CMakeLists.txt
===
--- lldb/unittests/Process/CMakeLists.txt
+++ lldb/unittests/Process/CMakeLists.txt
@@ -1,4 +1,7 @@
 add_subdirectory(gdb-remote)
+if (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
+  add_subdirectory(FreeBSD)
+endif()
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
   add_subdirectory(Linux)
   add_subdirectory(POSIX)
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
===
--- lldb/source/Plugins/Process/FreeBSDRem

[Lldb-commits] [PATCH] D91216: [lldb] [Process/FreeBSDRemote] Access GPR via reginfo offsets

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

mgorny updated this revision to Diff 304448.
mgorny added a comment.

Add a helper `ASSERT_REG` macro.


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

https://reviews.llvm.org/D91216

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
  lldb/unittests/Process/CMakeLists.txt
  lldb/unittests/Process/FreeBSD/CMakeLists.txt
  lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp

Index: lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp
===
--- /dev/null
+++ lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp
@@ -0,0 +1,100 @@
+//===-- RegisterContextFreeBSDTests.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
+//
+//===--===//
+
+// clang-format off
+#include 
+#include 
+// clang-format on
+
+#include "gtest/gtest.h"
+
+#if defined(__x86_64__) || defined(__i386__)
+#include "lldb-x86-register-enums.h"
+#include "RegisterContextFreeBSD_i386.h"
+#endif
+#if defined(__x86_64__)
+#include "RegisterContextFreeBSD_x86_64.h"
+#endif
+
+using namespace lldb_private;
+
+#define ASSERT_REG(regname, offset, size)  \
+  {\
+const RegisterInfo *reg_info = ®_ctx.GetRegisterInfo()[lldb_##regname]; \
+EXPECT_EQ(reg_info->byte_offset, offset);  \
+EXPECT_EQ(reg_info->byte_size, size);  \
+  }
+
+#if defined(__x86_64__)
+
+#define ASSERT_GPR_X86_64(regname) \
+  ASSERT_REG(regname##_x86_64, offsetof(reg, r_##regname), \
+ sizeof(reg::r_##regname))
+
+TEST(RegisterContextFreeBSDTest, x86_64) {
+  ArchSpec arch{"x86_64-unknown-freebsd12.2"};
+  RegisterContextFreeBSD_x86_64 reg_ctx{arch};
+
+  ASSERT_GPR_X86_64(r15);
+  ASSERT_GPR_X86_64(r14);
+  ASSERT_GPR_X86_64(r13);
+  ASSERT_GPR_X86_64(r12);
+  ASSERT_GPR_X86_64(r11);
+  ASSERT_GPR_X86_64(r10);
+  ASSERT_GPR_X86_64(r9);
+  ASSERT_GPR_X86_64(r8);
+  ASSERT_GPR_X86_64(rdi);
+  ASSERT_GPR_X86_64(rsi);
+  ASSERT_GPR_X86_64(rbp);
+  ASSERT_GPR_X86_64(rbx);
+  ASSERT_GPR_X86_64(rdx);
+  ASSERT_GPR_X86_64(rcx);
+  ASSERT_GPR_X86_64(rax);
+  ASSERT_GPR_X86_64(fs);
+  ASSERT_GPR_X86_64(gs);
+  ASSERT_GPR_X86_64(es);
+  ASSERT_GPR_X86_64(ds);
+  ASSERT_GPR_X86_64(rip);
+  ASSERT_GPR_X86_64(cs);
+  ASSERT_GPR_X86_64(rflags);
+  ASSERT_GPR_X86_64(rsp);
+  ASSERT_GPR_X86_64(ss);
+}
+#endif
+
+#if defined(__i386__) || defined(__x86_64__)
+
+#if defined(__i386__)
+#define reg32 reg
+#endif
+#define ASSERT_GPR_I386(regname)   \
+  ASSERT_REG(regname##_i386, offsetof(reg32, r_##regname), \
+ sizeof(reg32::r_##regname))
+
+TEST(RegisterContextFreeBSDTest, i386) {
+  ArchSpec arch{"i686-unknown-freebsd12.2"};
+  RegisterContextFreeBSD_i386 reg_ctx{arch};
+
+  ASSERT_GPR_I386(fs);
+  ASSERT_GPR_I386(es);
+  ASSERT_GPR_I386(ds);
+  ASSERT_GPR_I386(edi);
+  ASSERT_GPR_I386(esi);
+  ASSERT_GPR_I386(ebp);
+  ASSERT_GPR_I386(ebx);
+  ASSERT_GPR_I386(edx);
+  ASSERT_GPR_I386(ecx);
+  ASSERT_GPR_I386(eax);
+  ASSERT_GPR_I386(eip);
+  ASSERT_GPR_I386(cs);
+  ASSERT_GPR_I386(eflags);
+  ASSERT_GPR_I386(esp);
+  ASSERT_GPR_I386(ss);
+  ASSERT_GPR_I386(gs);
+}
+#endif
Index: lldb/unittests/Process/FreeBSD/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Process/FreeBSD/CMakeLists.txt
@@ -0,0 +1,9 @@
+add_lldb_unittest(RegisterContextFreeBSDTests
+  RegisterContextFreeBSDTests.cpp
+
+  LINK_LIBS
+lldbPluginProcessUtility
+  )
+
+target_include_directories(RegisterContextFreeBSDTests PRIVATE
+  ${LLDB_SOURCE_DIR}/source/Plugins/Process/Utility)
Index: lldb/unittests/Process/CMakeLists.txt
===
--- lldb/unittests/Process/CMakeLists.txt
+++ lldb/unittests/Process/CMakeLists.txt
@@ -1,4 +1,7 @@
 add_subdirectory(gdb-remote)
+if (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
+  add_subdirectory(FreeBSD)
+endif()
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
   add_subdirectory(Linux)
   add_subdirectory(POSIX)
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
@@ -18,6 +18,8 @

[Lldb-commits] [PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting

[Nathan James via Phabricator via lldb-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:636
+// Filter out any reformat fixits, we don't handle these.
+// FIXME: Can we?
+llvm::erase_if(FixIts,

kadircet wrote:
> in theory yes, as we have access to source manager, we can fetch file 
> contents and create formatted  replacements (see `cleanupAndFormat`). but 
> formatting those fixes can imply significant delays on clangd's diagnostic 
> cycles (if there are many of those), that's the reason why we currently don't 
> format fixits.
I mean if clangd is extended to support async code actions for diagnostics 
instead of just using the inline extension. Having said that, if that were the 
case, this would probably not be where that support is implemented.



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:637
+// FIXME: Can we?
+llvm::erase_if(FixIts,
+   [](const FixItHint &Fix) { return Fix.isReformatFixit(); });

kadircet wrote:
> rather than doing an extra loop, can we just skip those in the for loop below 
> ?
We could, however that would result in messier code further down when trying to 
build a synthetic message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91103

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

[Lldb-commits] [PATCH] D91248: [lldb] [Process/FreeBSDRemote] Access FPR via RegisterInfo offsets

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

mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
mgorny requested review of this revision.

Use offset-based method to access base x87 FPU registers, using offsets
relative to the position of 'struct FPR', as determined by the location
of first register in it (fctrl).  Change m_fpr to use a fixed-size array
matching FXSAVE size (512 bytes).  Add unit tests for verifying
RegisterInfo offsets and sizes against the FXSAVE layout.


https://reviews.llvm.org/D91248

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
  lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp

Index: lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp
===
--- lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp
+++ lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp
@@ -26,10 +26,13 @@
 #define ASSERT_REG(regname, offset, size)  \
   {\
 const RegisterInfo *reg_info = ®_ctx.GetRegisterInfo()[lldb_##regname]; \
-EXPECT_EQ(reg_info->byte_offset, offset);  \
-EXPECT_EQ(reg_info->byte_size, size);  \
+EXPECT_EQ(reg_info->byte_offset, static_cast(offset)); \
+EXPECT_EQ(reg_info->byte_size, static_cast(size)); \
   }
 
+#define ASSERT_FPR(regname, offset, size)  \
+  ASSERT_REG(regname, offset + fpr_offset, size)
+
 #if defined(__x86_64__)
 
 #define ASSERT_GPR_X86_64(regname) \
@@ -64,6 +67,57 @@
   ASSERT_GPR_X86_64(rflags);
   ASSERT_GPR_X86_64(rsp);
   ASSERT_GPR_X86_64(ss);
+
+  // fctrl is the first FPR field, it is used to determine offset of the whole
+  // FPR struct
+  size_t fpr_offset = reg_ctx.GetRegisterInfo()[lldb_fctrl_x86_64].byte_offset;
+
+  // assert against FXSAVE struct
+  ASSERT_FPR(fctrl_x86_64, 0x00, 2);
+  ASSERT_FPR(fstat_x86_64, 0x02, 2);
+  // TODO: This is a known bug, abridged ftag should is 8 bits in length.
+  ASSERT_FPR(ftag_x86_64, 0x04, 2);
+  ASSERT_FPR(fop_x86_64, 0x06, 2);
+  // NB: Technically fiseg/foseg are 16-bit long and the higher 16 bits
+  // are reserved.  However, we use them to access/recombine 64-bit FIP/FDP.
+  ASSERT_FPR(fioff_x86_64, 0x08, 4);
+  ASSERT_FPR(fiseg_x86_64, 0x0C, 4);
+  ASSERT_FPR(fooff_x86_64, 0x10, 4);
+  ASSERT_FPR(foseg_x86_64, 0x14, 4);
+  ASSERT_FPR(mxcsr_x86_64, 0x18, 4);
+  ASSERT_FPR(mxcsrmask_x86_64, 0x1C, 4);
+  ASSERT_FPR(st0_x86_64, 0x20, 10);
+  ASSERT_FPR(st1_x86_64, 0x30, 10);
+  ASSERT_FPR(st2_x86_64, 0x40, 10);
+  ASSERT_FPR(st3_x86_64, 0x50, 10);
+  ASSERT_FPR(st4_x86_64, 0x60, 10);
+  ASSERT_FPR(st5_x86_64, 0x70, 10);
+  ASSERT_FPR(st6_x86_64, 0x80, 10);
+  ASSERT_FPR(st7_x86_64, 0x90, 10);
+  ASSERT_FPR(mm0_x86_64, 0x20, 8);
+  ASSERT_FPR(mm1_x86_64, 0x30, 8);
+  ASSERT_FPR(mm2_x86_64, 0x40, 8);
+  ASSERT_FPR(mm3_x86_64, 0x50, 8);
+  ASSERT_FPR(mm4_x86_64, 0x60, 8);
+  ASSERT_FPR(mm5_x86_64, 0x70, 8);
+  ASSERT_FPR(mm6_x86_64, 0x80, 8);
+  ASSERT_FPR(mm7_x86_64, 0x90, 8);
+  ASSERT_FPR(xmm0_x86_64, 0xA0, 16);
+  ASSERT_FPR(xmm1_x86_64, 0xB0, 16);
+  ASSERT_FPR(xmm2_x86_64, 0xC0, 16);
+  ASSERT_FPR(xmm3_x86_64, 0xD0, 16);
+  ASSERT_FPR(xmm4_x86_64, 0xE0, 16);
+  ASSERT_FPR(xmm5_x86_64, 0xF0, 16);
+  ASSERT_FPR(xmm6_x86_64, 0x100, 16);
+  ASSERT_FPR(xmm7_x86_64, 0x110, 16);
+  ASSERT_FPR(xmm8_x86_64, 0x120, 16);
+  ASSERT_FPR(xmm9_x86_64, 0x130, 16);
+  ASSERT_FPR(xmm10_x86_64, 0x140, 16);
+  ASSERT_FPR(xmm11_x86_64, 0x150, 16);
+  ASSERT_FPR(xmm12_x86_64, 0x160, 16);
+  ASSERT_FPR(xmm13_x86_64, 0x170, 16);
+  ASSERT_FPR(xmm14_x86_64, 0x180, 16);
+  ASSERT_FPR(xmm15_x86_64, 0x190, 16);
 }
 #endif
 
@@ -96,5 +150,48 @@
   ASSERT_GPR_I386(esp);
   ASSERT_GPR_I386(ss);
   ASSERT_GPR_I386(gs);
+
+  // fctrl is the first FPR field, it is used to determine offset of the whole
+  // FPR struct
+  size_t fpr_offset = reg_ctx.GetRegisterInfo()[lldb_fctrl_i386].byte_offset;
+
+  // assert against FXSAVE struct
+  ASSERT_FPR(fctrl_i386, 0x00, 2);
+  ASSERT_FPR(fstat_i386, 0x02, 2);
+  // TODO: This is a known bug, abridged ftag should is 8 bits in length.
+  ASSERT_FPR(ftag_i386, 0x04, 2);
+  ASSERT_FPR(fop_i386, 0x06, 2);
+  // NB: Technically fiseg/foseg are 16-bit long and the higher 16 bits
+  // are reserved.  However, we use them to access/recombine 64-bit FIP/FDP.
+  ASSERT_FPR(fioff_i386, 0x08, 4);
+  ASSERT_FPR(fiseg_i386, 0x0C, 4);
+  ASSERT_FPR(fooff_i386, 0x10, 4);
+  ASSERT_FPR(foseg_i386, 0x14, 4);
+  ASSERT_FPR(mxcsr_i386, 0x18, 4);
+  ASSERT_FPR(mxcsrmask_i386, 0x1C, 4);
+  ASSERT_FPR(st0_i386, 0x20, 10);
+  ASSERT_FPR(st1_i386, 0x30, 10);
+  ASSERT_FPR(st2_i386, 0x40, 10);
+  ASSERT_FPR(

[Lldb-commits] [PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting

[Nathan James via Phabricator via lldb-commits]

njames93 updated this revision to Diff 304477.
njames93 marked 2 inline comments as done.
njames93 added a comment.

Removed the first loop for clangd diagnostic, turns out it didnt make the 
following code that much messier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91103

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clangd/Diagnostics.cpp
  clang/include/clang/Basic/Diagnostic.h
  clang/include/clang/Basic/SourceLocation.h
  clang/include/clang/Tooling/Core/Replacement.h
  clang/lib/Frontend/DiagnosticRenderer.cpp
  clang/lib/Frontend/Rewrite/FixItRewriter.cpp
  clang/lib/Tooling/Core/Replacement.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1167,9 +1167,10 @@
   // This is cobbed from clang::Rewrite::FixItRewriter.
   if (fixit.CodeToInsert.empty()) {
 if (fixit.InsertFromRange.isValid()) {
-  commit.insertFromRange(fixit.RemoveRange.getBegin(),
- fixit.InsertFromRange, /*afterToken=*/false,
- fixit.BeforePreviousInsertions);
+  if (!fixit.isReformatFixit())
+commit.insertFromRange(fixit.RemoveRange.getBegin(),
+   fixit.InsertFromRange, /*afterToken=*/false,
+   fixit.BeforePreviousInsertions);
   return;
 }
 commit.remove(fixit.RemoveRange);
Index: clang/lib/Tooling/Core/Replacement.cpp
===
--- clang/lib/Tooling/Core/Replacement.cpp
+++ clang/lib/Tooling/Core/Replacement.cpp
@@ -22,6 +22,7 @@
 #include "clang/Rewrite/Core/RewriteBuffer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
@@ -42,32 +43,57 @@
 
 static const char * const InvalidLocation = "";
 
-Replacement::Replacement() : FilePath(InvalidLocation) {}
+FileRange::FileRange() : FilePath(InvalidLocation), RangeInFile(0, 0) {}
+
+FileRange::FileRange(StringRef FilePath, Range RangeInFile)
+: FilePath(FilePath), RangeInFile(RangeInFile) {}
+FileRange::FileRange(StringRef FilePath, unsigned Offset, unsigned Length)
+: FileRange(FilePath, Range(Offset, Length)) {}
+
+FileRange::FileRange(const SourceManager &Sources, SourceLocation Start,
+ unsigned Length) {
+  setFromSourceLocation(Sources, Start, Length);
+}
+
+FileRange::FileRange(const SourceManager &Sources, const CharSourceRange &Range,
+ const LangOptions &LangOpts) {
+  setFromSourceRange(Sources, Range, LangOpts);
+}
+
+bool FileRange::isValid() const { return FilePath != InvalidLocation; }
+
+void FileRange::setFromSourceLocation(const SourceManager &Sources,
+  SourceLocation Start, unsigned Length) {
+  const std::pair DecomposedLocation =
+  Sources.getDecomposedLoc(Start);
+  const FileEntry *Entry = Sources.getFileEntryForID(DecomposedLocation.first);
+  this->FilePath = std::string(Entry ? Entry->getName() : InvalidLocation);
+  this->RangeInFile = {DecomposedLocation.second, Length};
+}
+
+Replacement::Replacement() : ReplacementRange() {}
+
+Replacement::Replacement(FileRange FileRange, StringRef ReplacementText)
+: ReplacementRange(FileRange), ReplacementText(ReplacementText) {}
 
 Replacement::Replacement(StringRef FilePath, unsigned Offset, unsigned Length,
  StringRef ReplacementText)
-: FilePath(std::string(FilePath)), ReplacementRange(Offset, Length),
-  ReplacementText(std::string(ReplacementText)) {}
+: Replacement(FileRange(FilePath, Offset, Length), ReplacementText) {}
 
 Replacement::Replacement(const SourceManager &Sources, SourceLocation Start,
- unsigned Length, StringRef ReplacementText) {
-  setFromSourceLocation(Sources, Start, Length, ReplacementText);
-}
+ unsigned Length, StringRef ReplacementText)
+: Replacement(FileRange(Sources, Start, Length), ReplacementText) {}
 
 Replacement::Replacement(const SourceManager &Sources,
  const CharSourceRange &Range,
- StringRef ReplacementText,
- const LangOptions &LangOpts) {
-  setFromSourceRange(Sources, Range, ReplacementText, LangOpts);
-}
+ StringRef ReplacementText, const LangOptions &LangOpts)
+: Replacement(FileRange(Sources, Range, LangOpts), ReplacementText) {}
 
-b

[Lldb-commits] [PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting

[Kadir Cetinkaya via Phabricator via lldb-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:677
 // If requested and possible, create a message like "change 'foo' to 
'bar'".
-if (SyntheticMessage && FixIts.size() == 1) {
-  const auto &FixIt = FixIts.front();
+if (SyntheticMessage && *SingleFixIt) {
+  const auto &FixIt = **SingleFixIt;

nit: you can check `Edits.size() == 1` here, `s/SingleFixIt/FixItForLastEdit` 
and get rid of the optional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91103

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

[Lldb-commits] [PATCH] D91248: [lldb] [Process/FreeBSDRemote] Access FPR via RegisterInfo offsets

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

mgorny updated this revision to Diff 304479.
mgorny added a comment.

Rename `ASSERT_FPR` to `ASSERT_OFF` since it's a more generic offset-based 
assert.


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

https://reviews.llvm.org/D91248

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
  lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp

Index: lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp
===
--- lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp
+++ lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp
@@ -26,10 +26,13 @@
 #define ASSERT_REG(regname, offset, size)  \
   {\
 const RegisterInfo *reg_info = ®_ctx.GetRegisterInfo()[lldb_##regname]; \
-EXPECT_EQ(reg_info->byte_offset, offset);  \
-EXPECT_EQ(reg_info->byte_size, size);  \
+EXPECT_EQ(reg_info->byte_offset, static_cast(offset)); \
+EXPECT_EQ(reg_info->byte_size, static_cast(size)); \
   }
 
+#define ASSERT_OFF(regname, offset, size)  \
+  ASSERT_REG(regname, offset + base_offset, size)
+
 #if defined(__x86_64__)
 
 #define ASSERT_GPR_X86_64(regname) \
@@ -64,6 +67,57 @@
   ASSERT_GPR_X86_64(rflags);
   ASSERT_GPR_X86_64(rsp);
   ASSERT_GPR_X86_64(ss);
+
+  // fctrl is the first FPR field, it is used to determine offset of the whole
+  // FPR struct
+  size_t base_offset = reg_ctx.GetRegisterInfo()[lldb_fctrl_x86_64].byte_offset;
+
+  // assert against FXSAVE struct
+  ASSERT_OFF(fctrl_x86_64, 0x00, 2);
+  ASSERT_OFF(fstat_x86_64, 0x02, 2);
+  // TODO: This is a known bug, abridged ftag should is 8 bits in length.
+  ASSERT_OFF(ftag_x86_64, 0x04, 2);
+  ASSERT_OFF(fop_x86_64, 0x06, 2);
+  // NB: Technically fiseg/foseg are 16-bit long and the higher 16 bits
+  // are reserved.  However, we use them to access/recombine 64-bit FIP/FDP.
+  ASSERT_OFF(fioff_x86_64, 0x08, 4);
+  ASSERT_OFF(fiseg_x86_64, 0x0C, 4);
+  ASSERT_OFF(fooff_x86_64, 0x10, 4);
+  ASSERT_OFF(foseg_x86_64, 0x14, 4);
+  ASSERT_OFF(mxcsr_x86_64, 0x18, 4);
+  ASSERT_OFF(mxcsrmask_x86_64, 0x1C, 4);
+  ASSERT_OFF(st0_x86_64, 0x20, 10);
+  ASSERT_OFF(st1_x86_64, 0x30, 10);
+  ASSERT_OFF(st2_x86_64, 0x40, 10);
+  ASSERT_OFF(st3_x86_64, 0x50, 10);
+  ASSERT_OFF(st4_x86_64, 0x60, 10);
+  ASSERT_OFF(st5_x86_64, 0x70, 10);
+  ASSERT_OFF(st6_x86_64, 0x80, 10);
+  ASSERT_OFF(st7_x86_64, 0x90, 10);
+  ASSERT_OFF(mm0_x86_64, 0x20, 8);
+  ASSERT_OFF(mm1_x86_64, 0x30, 8);
+  ASSERT_OFF(mm2_x86_64, 0x40, 8);
+  ASSERT_OFF(mm3_x86_64, 0x50, 8);
+  ASSERT_OFF(mm4_x86_64, 0x60, 8);
+  ASSERT_OFF(mm5_x86_64, 0x70, 8);
+  ASSERT_OFF(mm6_x86_64, 0x80, 8);
+  ASSERT_OFF(mm7_x86_64, 0x90, 8);
+  ASSERT_OFF(xmm0_x86_64, 0xA0, 16);
+  ASSERT_OFF(xmm1_x86_64, 0xB0, 16);
+  ASSERT_OFF(xmm2_x86_64, 0xC0, 16);
+  ASSERT_OFF(xmm3_x86_64, 0xD0, 16);
+  ASSERT_OFF(xmm4_x86_64, 0xE0, 16);
+  ASSERT_OFF(xmm5_x86_64, 0xF0, 16);
+  ASSERT_OFF(xmm6_x86_64, 0x100, 16);
+  ASSERT_OFF(xmm7_x86_64, 0x110, 16);
+  ASSERT_OFF(xmm8_x86_64, 0x120, 16);
+  ASSERT_OFF(xmm9_x86_64, 0x130, 16);
+  ASSERT_OFF(xmm10_x86_64, 0x140, 16);
+  ASSERT_OFF(xmm11_x86_64, 0x150, 16);
+  ASSERT_OFF(xmm12_x86_64, 0x160, 16);
+  ASSERT_OFF(xmm13_x86_64, 0x170, 16);
+  ASSERT_OFF(xmm14_x86_64, 0x180, 16);
+  ASSERT_OFF(xmm15_x86_64, 0x190, 16);
 }
 #endif
 
@@ -96,5 +150,48 @@
   ASSERT_GPR_I386(esp);
   ASSERT_GPR_I386(ss);
   ASSERT_GPR_I386(gs);
+
+  // fctrl is the first FPR field, it is used to determine offset of the whole
+  // FPR struct
+  size_t base_offset = reg_ctx.GetRegisterInfo()[lldb_fctrl_i386].byte_offset;
+
+  // assert against FXSAVE struct
+  ASSERT_OFF(fctrl_i386, 0x00, 2);
+  ASSERT_OFF(fstat_i386, 0x02, 2);
+  // TODO: This is a known bug, abridged ftag should is 8 bits in length.
+  ASSERT_OFF(ftag_i386, 0x04, 2);
+  ASSERT_OFF(fop_i386, 0x06, 2);
+  // NB: Technically fiseg/foseg are 16-bit long and the higher 16 bits
+  // are reserved.  However, we use them to access/recombine 64-bit FIP/FDP.
+  ASSERT_OFF(fioff_i386, 0x08, 4);
+  ASSERT_OFF(fiseg_i386, 0x0C, 4);
+  ASSERT_OFF(fooff_i386, 0x10, 4);
+  ASSERT_OFF(foseg_i386, 0x14, 4);
+  ASSERT_OFF(mxcsr_i386, 0x18, 4);
+  ASSERT_OFF(mxcsrmask_i386, 0x1C, 4);
+  ASSERT_OFF(st0_i386, 0x20, 10);
+  ASSERT_OFF(st1_i386, 0x30, 10);
+  ASSERT_OFF(st2_i386, 0x40, 10);
+  ASSERT_OFF(st3_i386, 0x50, 10);
+  ASSERT_OFF(st4_i386, 0x60, 10);
+  ASSERT_OFF(st5_i386, 0x70, 10);
+  ASSERT_OFF(st6_i386, 0x80, 10);
+  ASSERT_OFF(st7_i386, 0x90, 10);
+  ASSERT_OFF(mm0_i386, 0x20, 8);
+  ASSERT_OFF(mm1_i386, 0x30, 8);
+  ASSERT_OFF

[Lldb-commits] [PATCH] D91254: [lldb] [Process/Utility] Fix DR offsets for FreeBSD

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

mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
Herald added a subscriber: arichardson.
mgorny requested review of this revision.

Fix Debug Register offsets to be specified relatively to UserArea
on FreeBSD/amd64 and FreeBSD/i386, and add them to UserArea on i386.
This fixes overlapping GPRs and DRs in gdb-remote protocol, making it
impossible to correctly get and set debug registers from the LLDB
client.


https://reviews.llvm.org/D91254

Files:
  lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp


Index: lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
@@ -59,7 +59,9 @@
   DBG dbg;
 };
 
-#define DR_OFFSET(reg_index) (LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
+#define DR_OFFSET(reg_index)   
\
+  (LLVM_EXTENSION offsetof(UserArea, dbg) +
\
+   LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
 
 // Include RegisterInfos_x86_64 to declare our g_register_infos_x86_64
 // structure.
Index: lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
@@ -35,7 +35,7 @@
   uint32_t gs;
 };
 
-struct dbreg {
+struct DBG {
   uint32_t dr[8]; /* debug registers */
   /* Index 0-3: debug address registers */
   /* Index 4-5: reserved */
@@ -48,10 +48,13 @@
 struct UserArea {
   GPR gpr;
   FPR_i386 i387;
+  DBG dbg;
 };
 
 #define DR_SIZE sizeof(uint32_t)
-#define DR_OFFSET(reg_index) (LLVM_EXTENSION offsetof(dbreg, dr[reg_index]))
+#define DR_OFFSET(reg_index)   
\
+  (LLVM_EXTENSION offsetof(UserArea, dbg) +
\
+   LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
 
 // Include RegisterInfos_i386 to declare our g_register_infos_i386 structure.
 #define DECLARE_REGISTER_INFOS_I386_STRUCT


Index: lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
@@ -59,7 +59,9 @@
   DBG dbg;
 };
 
-#define DR_OFFSET(reg_index) (LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
+#define DR_OFFSET(reg_index)   \
+  (LLVM_EXTENSION offsetof(UserArea, dbg) +\
+   LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
 
 // Include RegisterInfos_x86_64 to declare our g_register_infos_x86_64
 // structure.
Index: lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
@@ -35,7 +35,7 @@
   uint32_t gs;
 };
 
-struct dbreg {
+struct DBG {
   uint32_t dr[8]; /* debug registers */
   /* Index 0-3: debug address registers */
   /* Index 4-5: reserved */
@@ -48,10 +48,13 @@
 struct UserArea {
   GPR gpr;
   FPR_i386 i387;
+  DBG dbg;
 };
 
 #define DR_SIZE sizeof(uint32_t)
-#define DR_OFFSET(reg_index) (LLVM_EXTENSION offsetof(dbreg, dr[reg_index]))
+#define DR_OFFSET(reg_index)   \
+  (LLVM_EXTENSION offsetof(UserArea, dbg) +\
+   LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
 
 // Include RegisterInfos_i386 to declare our g_register_infos_i386 structure.
 #define DECLARE_REGISTER_INFOS_I386_STRUCT
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91264: [lldb] [test] Add a minimal test for x86 dbreg reading

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

mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
Herald added a subscriber: pengfei.
mgorny requested review of this revision.

Add a test verifying that after the 'watchpoint' command, new values
of x86 debug registers can be read back correctly.  The primary purpose
of this test is to catch broken DRn reading and help debugging it.


https://reviews.llvm.org/D91264

Files:
  lldb/test/Shell/Register/Inputs/x86-db-read.cpp
  lldb/test/Shell/Register/x86-db-read.test


Index: lldb/test/Shell/Register/x86-db-read.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-db-read.test
@@ -0,0 +1,35 @@
+# XFAIL: system-windows
+# REQUIRES: native && (target-x86 || target-x86_64) && can-set-dbregs
+# RUN: %clangxx_host -g %p/Inputs/x86-db-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+# CHECK: Process {{[0-9]+}} stopped
+
+watchpoint set variable -w write g_8w
+# CHECK: Watchpoint created: Watchpoint 1: addr = 0x{{[0-9a-f]*}} size = 1 
state = enabled type = w
+watchpoint set variable -w read_write g_16rw
+# CHECK: Watchpoint created: Watchpoint 2: addr = 0x{{[0-9a-f]*}} size = 2 
state = enabled type = rw
+watchpoint set variable -w write g_32w
+# CHECK: Watchpoint created: Watchpoint 3: addr = 0x{{[0-9a-f]*}} size = 4 
state = enabled type = w
+watchpoint set variable -w read_write g_32rw
+# CHECK: Watchpoint created: Watchpoint 4: addr = 0x{{[0-9a-f]*}} size = 4 
state = enabled type = rw
+
+print &g_8w
+# CHECK: (uint8_t *) $0 = [[VAR8W:0x[0-9a-f]*]]
+print &g_16rw
+# CHECK: (uint16_t *) $1 = [[VAR16RW:0x[0-9a-f]*]]
+print &g_32w
+# CHECK: (uint32_t *) $2 = [[VAR32W:0x[0-9a-f]*]]
+print &g_32rw
+# CHECK: (uint32_t *) $3 = [[VAR64RW:0x[0-9a-f]*]]
+
+register read --all
+# CHECK-DAG: dr0 = [[VAR8W]]
+# CHECK-DAG: dr1 = [[VAR16RW]]
+# CHECK-DAG: dr2 = [[VAR32W]]
+# CHECK-DAG: dr3 = [[VAR64RW]]
+# CHECK-DAG: dr6 = 0x{{()?}}0ff0
+# CHECK-DAG: dr7 = 0x{{()?}}fd7104aa
+
+process continue
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/Inputs/x86-db-read.cpp
===
--- /dev/null
+++ lldb/test/Shell/Register/Inputs/x86-db-read.cpp
@@ -0,0 +1,12 @@
+#include 
+#include 
+
+uint8_t g_8w;
+uint16_t g_16rw;
+uint32_t g_32w;
+uint32_t g_32rw;
+
+int main() {
+  ::raise(SIGSTOP);
+  return 0;
+}


Index: lldb/test/Shell/Register/x86-db-read.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-db-read.test
@@ -0,0 +1,35 @@
+# XFAIL: system-windows
+# REQUIRES: native && (target-x86 || target-x86_64) && can-set-dbregs
+# RUN: %clangxx_host -g %p/Inputs/x86-db-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+# CHECK: Process {{[0-9]+}} stopped
+
+watchpoint set variable -w write g_8w
+# CHECK: Watchpoint created: Watchpoint 1: addr = 0x{{[0-9a-f]*}} size = 1 state = enabled type = w
+watchpoint set variable -w read_write g_16rw
+# CHECK: Watchpoint created: Watchpoint 2: addr = 0x{{[0-9a-f]*}} size = 2 state = enabled type = rw
+watchpoint set variable -w write g_32w
+# CHECK: Watchpoint created: Watchpoint 3: addr = 0x{{[0-9a-f]*}} size = 4 state = enabled type = w
+watchpoint set variable -w read_write g_32rw
+# CHECK: Watchpoint created: Watchpoint 4: addr = 0x{{[0-9a-f]*}} size = 4 state = enabled type = rw
+
+print &g_8w
+# CHECK: (uint8_t *) $0 = [[VAR8W:0x[0-9a-f]*]]
+print &g_16rw
+# CHECK: (uint16_t *) $1 = [[VAR16RW:0x[0-9a-f]*]]
+print &g_32w
+# CHECK: (uint32_t *) $2 = [[VAR32W:0x[0-9a-f]*]]
+print &g_32rw
+# CHECK: (uint32_t *) $3 = [[VAR64RW:0x[0-9a-f]*]]
+
+register read --all
+# CHECK-DAG: dr0 = [[VAR8W]]
+# CHECK-DAG: dr1 = [[VAR16RW]]
+# CHECK-DAG: dr2 = [[VAR32W]]
+# CHECK-DAG: dr3 = [[VAR64RW]]
+# CHECK-DAG: dr6 = 0x{{()?}}0ff0
+# CHECK-DAG: dr7 = 0x{{()?}}fd7104aa
+
+process continue
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/Inputs/x86-db-read.cpp
===
--- /dev/null
+++ lldb/test/Shell/Register/Inputs/x86-db-read.cpp
@@ -0,0 +1,12 @@
+#include 
+#include 
+
+uint8_t g_8w;
+uint16_t g_16rw;
+uint32_t g_32w;
+uint32_t g_32rw;
+
+int main() {
+  ::raise(SIGSTOP);
+  return 0;
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91268: [lldb] [Process/FreeBSDRemote] Access debug registers via offsets

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

mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski.
Herald added a subscriber: pengfei.
mgorny requested review of this revision.

Use offset-based method to access x86 debug registers.  This also
involves adding a test for the correctness of these offsets, and making
GetDR() method of NativeRegisterContextWatchpoint_x86 public to avoid
duplicate code.


https://reviews.llvm.org/D91268

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
  lldb/source/Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h
  lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp

Index: lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp
===
--- lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp
+++ lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp
@@ -38,6 +38,8 @@
 #define ASSERT_GPR_X86_64(regname) \
   ASSERT_REG(regname##_x86_64, offsetof(reg, r_##regname), \
  sizeof(reg::r_##regname))
+#define ASSERT_DBR_X86_64(num) \
+  ASSERT_OFF(dr##num##_x86_64, offsetof(dbreg, dr[num]), sizeof(dbreg::dr[num]))
 
 TEST(RegisterContextFreeBSDTest, x86_64) {
   ArchSpec arch{"x86_64-unknown-freebsd12.2"};
@@ -118,6 +120,16 @@
   ASSERT_OFF(xmm13_x86_64, 0x170, 16);
   ASSERT_OFF(xmm14_x86_64, 0x180, 16);
   ASSERT_OFF(xmm15_x86_64, 0x190, 16);
+
+  base_offset = reg_ctx.GetRegisterInfo()[lldb_dr0_x86_64].byte_offset;
+  ASSERT_DBR_X86_64(0);
+  ASSERT_DBR_X86_64(1);
+  ASSERT_DBR_X86_64(2);
+  ASSERT_DBR_X86_64(3);
+  ASSERT_DBR_X86_64(4);
+  ASSERT_DBR_X86_64(5);
+  ASSERT_DBR_X86_64(6);
+  ASSERT_DBR_X86_64(7);
 }
 #endif
 
@@ -125,10 +137,14 @@
 
 #if defined(__i386__)
 #define reg32 reg
+#define dbreg32 dbreg
 #endif
 #define ASSERT_GPR_I386(regname)   \
   ASSERT_REG(regname##_i386, offsetof(reg32, r_##regname), \
  sizeof(reg32::r_##regname))
+#define ASSERT_DBR_I386(num)   \
+  ASSERT_OFF(dr##num##_i386, offsetof(dbreg32, dr[num]),   \
+ sizeof(dbreg32::dr[num]))
 
 TEST(RegisterContextFreeBSDTest, i386) {
   ArchSpec arch{"i686-unknown-freebsd12.2"};
@@ -193,5 +209,15 @@
   ASSERT_OFF(xmm5_i386, 0xF0, 16);
   ASSERT_OFF(xmm6_i386, 0x100, 16);
   ASSERT_OFF(xmm7_i386, 0x110, 16);
+
+  base_offset = reg_ctx.GetRegisterInfo()[lldb_dr0_i386].byte_offset;
+  ASSERT_DBR_I386(0);
+  ASSERT_DBR_I386(1);
+  ASSERT_DBR_I386(2);
+  ASSERT_DBR_I386(3);
+  ASSERT_DBR_I386(4);
+  ASSERT_DBR_I386(5);
+  ASSERT_DBR_I386(6);
+  ASSERT_DBR_I386(7);
 }
 #endif
Index: lldb/source/Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h
===
--- lldb/source/Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h
+++ lldb/source/Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h
@@ -40,7 +40,6 @@
 
   uint32_t NumSupportedHardwareWatchpoints() override;
 
-private:
   const RegisterInfo *GetDR(int num) const;
 };
 
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
@@ -71,17 +71,17 @@
   // Private member variables.
   std::array m_gpr;
   std::array m_fpr; // FXSAVE
-  struct dbreg m_dbr;
+  std::array m_dbr;
   std::vector m_xsave;
   std::array m_xsave_offsets;
 
   llvm::Optional GetSetForNativeRegNum(int reg_num) const;
-  int GetDR(int num) const;
 
   Status ReadRegisterSet(uint32_t set);
   Status WriteRegisterSet(uint32_t set);
 
   size_t GetFPROffset() const;
+  size_t GetDBROffset() const;
 };
 
 } // namespace process_freebsd
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -247,8 +247,7 @@
 NativeRegisterContextFreeBSD_x86_64::NativeRegisterContextFreeBSD_x86_64(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
 : NativeRegisterContextRegisterInfo(
-  native_thread, CreateRegisterInfoInterface(target_arch)),
-  m_dbr() {
+  native_thread, CreateRegisterInfoInterface(target_arch)) {
   assert(m_gpr.size() == GetRegisterInfoInterface().GetGPRSize());
 }
 
@@ -296,15 +295,6 @@
 return lldb_bndcfgu_x86_64;
   case lldb_b

[Lldb-commits] [lldb] d9624f4 - Revert "[ThreadPlan] Add a test for `thread step-in -r`, NFC"

[Stella Stamenova via lldb-commits]

Author: Stella Stamenova
Date: 2020-11-11T09:09:43-08:00
New Revision: d9624f444807bdac92e37f85ab07db1eb8a2bdf2

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

LOG: Revert "[ThreadPlan] Add a test for `thread step-in -r`, NFC"

This reverts commit ae3640e386ccfbe0e984cc8c4b0399006ed835c7.

The new test is failing on the Windows LLDB buildbot.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/test/API/lang/c/stepping/main.c

Removed: 
lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py



diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 89c25eb55516..f7d59a8a065c 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -126,8 +126,6 @@
 
 SOURCE_DISPLAYED_CORRECTLY = "Source code displayed correctly"
 
-STEP_IN_SUCCEEDED = "Thread step-in succeeded"
-
 STEP_OUT_SUCCEEDED = "Thread step-out succeeded"
 
 STOPPED_DUE_TO_EXC_BAD_ACCESS = "Process should be stopped due to bad access 
exception"

diff  --git a/lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py 
b/lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py
deleted file mode 100644
index 0b59f625bd69..
--- a/lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py
+++ /dev/null
@@ -1,33 +0,0 @@
-"""
-Test thread step-in [ -r | --step-over-regexp ].
-"""
-
-
-
-import lldb
-from lldbsuite.test.lldbtest import *
-import lldbsuite.test.lldbutil as lldbutil
-
-
-class ThreadStepInAvoidRegexTestCase(TestBase):
-
-mydir = TestBase.compute_mydir(__file__)
-
-def setUp(self):
-TestBase.setUp(self)
-self.line2 = line_number('main.c', '// assignment to B2')
-
-def test_step_out_avoid_regexp(self):
-"""Exercise thread step-in -r"""
-self.build()
-lldbutil.run_to_source_breakpoint(self,
-'frame select 2, thread step-out while stopped',
-lldb.SBFileSpec('main.c'))
-
-# Now step in, skipping the frames for 'b' and 'a'.
-self.runCmd("thread step-in -r 'a'")
-
-# We should be at the assignment to B2.
-self.expect("thread backtrace", STEP_IN_SUCCEEDED,
-substrs=["stop reason = step in"],
-patterns=["frame #0.*main.c:%d" % self.line2])

diff  --git a/lldb/test/API/lang/c/stepping/main.c 
b/lldb/test/API/lang/c/stepping/main.c
index 8237ebf984c9..43b5cfab183a 100644
--- a/lldb/test/API/lang/c/stepping/main.c
+++ b/lldb/test/API/lang/c/stepping/main.c
@@ -39,7 +39,7 @@ int main (int argc, char const *argv[])
 {
 int A1 = a(1); // frame select 2, thread step-out while stopped at "c(1)"
 
-int B2 = b(2); // assignment to B2
+int B2 = b(2);
 
 int A3 = a(3); // frame select 1, thread step-out while stopped at "c(3)"
 



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

[Lldb-commits] [PATCH] D91193: [lldb] Fine tune expect() validation

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

Thanks for pointing this in the right direction @labath, @teemperor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91193

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

[Lldb-commits] [PATCH] D91206: [lldb] Switch expect to runCmd in TestRecursiveTypes (NFC)

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 304568.
kastiglione added a comment.

Remove bogus message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91206

Files:
  lldb/test/API/types/TestRecursiveTypes.py


Index: lldb/test/API/types/TestRecursiveTypes.py
===
--- lldb/test/API/types/TestRecursiveTypes.py
+++ lldb/test/API/types/TestRecursiveTypes.py
@@ -50,5 +50,5 @@
 
 self.runCmd("run", RUN_SUCCEEDED)
 
-self.expect("print tpi")
-self.expect("print *tpi")
+self.runCmd("print tpi")
+self.runCmd("print *tpi")


Index: lldb/test/API/types/TestRecursiveTypes.py
===
--- lldb/test/API/types/TestRecursiveTypes.py
+++ lldb/test/API/types/TestRecursiveTypes.py
@@ -50,5 +50,5 @@
 
 self.runCmd("run", RUN_SUCCEEDED)
 
-self.expect("print tpi")
-self.expect("print *tpi")
+self.runCmd("print tpi")
+self.runCmd("print *tpi")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] d9624f4 - Revert "[ThreadPlan] Add a test for `thread step-in -r`, NFC"

[Vedant Kumar via lldb-commits]

Thanks for the revert.

Do you know anyone actively working on ThreadPlan support for Windows?

vedant

> On Nov 11, 2020, at 9:10 AM, Stella Stamenova via lldb-commits 
>  wrote:
> 
> 
> Author: Stella Stamenova
> Date: 2020-11-11T09:09:43-08:00
> New Revision: d9624f444807bdac92e37f85ab07db1eb8a2bdf2
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/d9624f444807bdac92e37f85ab07db1eb8a2bdf2
> DIFF: 
> https://github.com/llvm/llvm-project/commit/d9624f444807bdac92e37f85ab07db1eb8a2bdf2.diff
> 
> LOG: Revert "[ThreadPlan] Add a test for `thread step-in -r`, NFC"
> 
> This reverts commit ae3640e386ccfbe0e984cc8c4b0399006ed835c7.
> 
> The new test is failing on the Windows LLDB buildbot.
> 
> Added: 
> 
> 
> Modified: 
>lldb/packages/Python/lldbsuite/test/lldbtest.py
>lldb/test/API/lang/c/stepping/main.c
> 
> Removed: 
>lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py
> 
> 
> 
> diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
> b/lldb/packages/Python/lldbsuite/test/lldbtest.py
> index 89c25eb55516..f7d59a8a065c 100644
> --- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
> +++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
> @@ -126,8 +126,6 @@
> 
> SOURCE_DISPLAYED_CORRECTLY = "Source code displayed correctly"
> 
> -STEP_IN_SUCCEEDED = "Thread step-in succeeded"
> -
> STEP_OUT_SUCCEEDED = "Thread step-out succeeded"
> 
> STOPPED_DUE_TO_EXC_BAD_ACCESS = "Process should be stopped due to bad access 
> exception"
> 
> diff  --git a/lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py 
> b/lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py
> deleted file mode 100644
> index 0b59f625bd69..
> --- a/lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -"""
> -Test thread step-in [ -r | --step-over-regexp ].
> -"""
> -
> -
> -
> -import lldb
> -from lldbsuite.test.lldbtest import *
> -import lldbsuite.test.lldbutil as lldbutil
> -
> -
> -class ThreadStepInAvoidRegexTestCase(TestBase):
> -
> -mydir = TestBase.compute_mydir(__file__)
> -
> -def setUp(self):
> -TestBase.setUp(self)
> -self.line2 = line_number('main.c', '// assignment to B2')
> -
> -def test_step_out_avoid_regexp(self):
> -"""Exercise thread step-in -r"""
> -self.build()
> -lldbutil.run_to_source_breakpoint(self,
> -'frame select 2, thread step-out while stopped',
> -lldb.SBFileSpec('main.c'))
> -
> -# Now step in, skipping the frames for 'b' and 'a'.
> -self.runCmd("thread step-in -r 'a'")
> -
> -# We should be at the assignment to B2.
> -self.expect("thread backtrace", STEP_IN_SUCCEEDED,
> -substrs=["stop reason = step in"],
> -patterns=["frame #0.*main.c:%d" % self.line2])
> 
> diff  --git a/lldb/test/API/lang/c/stepping/main.c 
> b/lldb/test/API/lang/c/stepping/main.c
> index 8237ebf984c9..43b5cfab183a 100644
> --- a/lldb/test/API/lang/c/stepping/main.c
> +++ b/lldb/test/API/lang/c/stepping/main.c
> @@ -39,7 +39,7 @@ int main (int argc, char const *argv[])
> {
> int A1 = a(1); // frame select 2, thread step-out while stopped at "c(1)"
> 
> -int B2 = b(2); // assignment to B2
> +int B2 = b(2);
> 
> int A3 = a(3); // frame select 1, thread step-out while stopped at "c(3)"
> 
> 
> 
> 
> ___
> 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] D91103: [tooling] Add support for fixits that indicate code will need reformatting

[Nathan James via Phabricator via lldb-commits]

njames93 updated this revision to Diff 304574.
njames93 marked an inline comment as done.
njames93 added a comment.

Address nit by replacing optional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91103

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clangd/Diagnostics.cpp
  clang/include/clang/Basic/Diagnostic.h
  clang/include/clang/Basic/SourceLocation.h
  clang/include/clang/Tooling/Core/Replacement.h
  clang/lib/Frontend/DiagnosticRenderer.cpp
  clang/lib/Frontend/Rewrite/FixItRewriter.cpp
  clang/lib/Tooling/Core/Replacement.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1167,9 +1167,10 @@
   // This is cobbed from clang::Rewrite::FixItRewriter.
   if (fixit.CodeToInsert.empty()) {
 if (fixit.InsertFromRange.isValid()) {
-  commit.insertFromRange(fixit.RemoveRange.getBegin(),
- fixit.InsertFromRange, /*afterToken=*/false,
- fixit.BeforePreviousInsertions);
+  if (!fixit.isReformatFixit())
+commit.insertFromRange(fixit.RemoveRange.getBegin(),
+   fixit.InsertFromRange, /*afterToken=*/false,
+   fixit.BeforePreviousInsertions);
   return;
 }
 commit.remove(fixit.RemoveRange);
Index: clang/lib/Tooling/Core/Replacement.cpp
===
--- clang/lib/Tooling/Core/Replacement.cpp
+++ clang/lib/Tooling/Core/Replacement.cpp
@@ -22,6 +22,7 @@
 #include "clang/Rewrite/Core/RewriteBuffer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
@@ -42,32 +43,57 @@
 
 static const char * const InvalidLocation = "";
 
-Replacement::Replacement() : FilePath(InvalidLocation) {}
+FileRange::FileRange() : FilePath(InvalidLocation), RangeInFile(0, 0) {}
+
+FileRange::FileRange(StringRef FilePath, Range RangeInFile)
+: FilePath(FilePath), RangeInFile(RangeInFile) {}
+FileRange::FileRange(StringRef FilePath, unsigned Offset, unsigned Length)
+: FileRange(FilePath, Range(Offset, Length)) {}
+
+FileRange::FileRange(const SourceManager &Sources, SourceLocation Start,
+ unsigned Length) {
+  setFromSourceLocation(Sources, Start, Length);
+}
+
+FileRange::FileRange(const SourceManager &Sources, const CharSourceRange &Range,
+ const LangOptions &LangOpts) {
+  setFromSourceRange(Sources, Range, LangOpts);
+}
+
+bool FileRange::isValid() const { return FilePath != InvalidLocation; }
+
+void FileRange::setFromSourceLocation(const SourceManager &Sources,
+  SourceLocation Start, unsigned Length) {
+  const std::pair DecomposedLocation =
+  Sources.getDecomposedLoc(Start);
+  const FileEntry *Entry = Sources.getFileEntryForID(DecomposedLocation.first);
+  this->FilePath = std::string(Entry ? Entry->getName() : InvalidLocation);
+  this->RangeInFile = {DecomposedLocation.second, Length};
+}
+
+Replacement::Replacement() : ReplacementRange() {}
+
+Replacement::Replacement(FileRange FileRange, StringRef ReplacementText)
+: ReplacementRange(FileRange), ReplacementText(ReplacementText) {}
 
 Replacement::Replacement(StringRef FilePath, unsigned Offset, unsigned Length,
  StringRef ReplacementText)
-: FilePath(std::string(FilePath)), ReplacementRange(Offset, Length),
-  ReplacementText(std::string(ReplacementText)) {}
+: Replacement(FileRange(FilePath, Offset, Length), ReplacementText) {}
 
 Replacement::Replacement(const SourceManager &Sources, SourceLocation Start,
- unsigned Length, StringRef ReplacementText) {
-  setFromSourceLocation(Sources, Start, Length, ReplacementText);
-}
+ unsigned Length, StringRef ReplacementText)
+: Replacement(FileRange(Sources, Start, Length), ReplacementText) {}
 
 Replacement::Replacement(const SourceManager &Sources,
  const CharSourceRange &Range,
- StringRef ReplacementText,
- const LangOptions &LangOpts) {
-  setFromSourceRange(Sources, Range, ReplacementText, LangOpts);
-}
+ StringRef ReplacementText, const LangOptions &LangOpts)
+: Replacement(FileRange(Sources, Range, LangOpts), ReplacementText) {}
 
-bool Replacement::isApplicable() const {
-  return FilePath != InvalidLocat

[Lldb-commits] [lldb] 21555ff - [intel-pt][trace] Implement a "get supported trace type" packet

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2020-11-11T10:35:58-08:00
New Revision: 21555fff4de811309ea7935f9cb65578c957d77f

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

LOG: [intel-pt][trace] Implement a "get supported trace type" packet

Depends on D89283.

The goal of this packet (jTraceGetSupportedType) is to be able to query the 
gdb-server for the tracing technology that can work for the current debuggeer, 
which can make the user experience simpler but allowing the user to simply type

  thread trace start

to start tracing the current thread without even telling the debugger to use 
"intel-pt", for example. Similarly, `thread trace start [args...]` would accept 
args beloging to the working trace type.

Also, if the user typed

  help thread trace start

We could directly show the help information of the trace type that is supported 
for the target, or mention instead that no tracing is supported, if that's the 
case.

I added some simple tests, besides, when I ran this on my machine with intel-pt 
support, I got

  $ process plugin packet send "jTraceSupportedType"
packet: jTraceSupportedType
  response: {"description":"Intel Processor Trace","pluginName":"intel-pt"}

On a machine without intel-pt support, I got

  $ process plugin packet send "jTraceSupportedType"
packet: jTraceSupportedType
  response: E00;

Reviewed By: clayborg, labath

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

Added: 
lldb/source/Utility/TraceOptions.cpp

Modified: 
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Host/common/NativeProcessProtocol.h
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Target/Trace.h
lldb/include/lldb/Utility/StringExtractorGDBRemote.h
lldb/include/lldb/Utility/TraceOptions.h
lldb/include/lldb/lldb-enumerations.h
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
lldb/source/Plugins/Process/Linux/ProcessorTrace.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Utility/CMakeLists.txt
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Removed: 




diff  --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index 276beedd047c..91f6a4d12c2e 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -234,9 +234,38 @@ This packet must be sent  _prior_ to sending a "A" packet.
 send packet: QListThreadsInStopReply
 read packet: OK
 
+//--
+// jLLDBTraceSupportedType
+//
+// BRIEF
+//  Get the processor tracing type supported by the gdb-server for the current
+//  inferior. Responses might be 
diff erent depending on the architecture and
+//  capabilities of the underlying OS.
+//
+//  The return packet is a JSON object with the following schema
+//
+//  {
+//"name": 
+//"description": 
+//  }
+//
+//  If no tracing technology is supported for the inferior, or no process is
+//  running, then an error should be returned.
+//
+// NOTE
+//  This packet is used by Trace plug-ins (see lldb_private::Trace.h) to
+//  do live tracing. Specifically, the name of the plug-in should match the 
name
+//  of the tracing technology returned by this packet.
+//--
+
+send packet: jLLDBTraceSupportedType
+read packet: {"name": , "description", }/E;A
+
 //--
 // jTraceStart:
 //
+// This packet is deprecated.
+//
 // BRIEF
 //  Packet for starting trace of type lldb::TraceType. The following
 //  parameters should be appended to the packet formatted as a JSON
@@ -286,6 +315,8 @@ read packet: /E;A
 //--
 // jTraceStop:
 //
+// This packet is deprecated.
+//
 // BRIEF
 //  Stop tracing instance with trace id , of course trace
 //  needs to be started before. The following parameters should be
@@ -320,6 +351,8 @@ read packet: /E;A
 //--
 // jTraceBufferRead:
 //
+// This packet is deprecated.
+//
 // BRIEF
 //  Packet for reading the trace for tracing ins

[Lldb-commits] [PATCH] D90490: [intel-pt][trace] Implement a "get supported trace type" packet

[walter erquinigo via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG21555fff4de8: [intel-pt][trace] Implement a "get 
supported trace type" packet (authored by wallace).

Changed prior to commit:
  https://reviews.llvm.org/D90490?vs=304232&id=304575#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90490

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/include/lldb/Utility/TraceOptions.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
  lldb/source/Plugins/Process/Linux/ProcessorTrace.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/source/Utility/TraceOptions.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -362,6 +362,63 @@
   EXPECT_FALSE(result.get().Success());
 }
 
+TEST_F(GDBRemoteCommunicationClientTest, SendTraceSupportedTypePacket) {
+  // Success response
+  {
+std::future> result = std::async(
+std::launch::async, [&] { return client.SendGetSupportedTraceType(); });
+
+HandlePacket(
+server, "jLLDBTraceSupportedType",
+R"({"name":"intel-pt","description":"Intel Processor Trace"}])");
+
+llvm::Expected trace_type_or_err = result.get();
+EXPECT_THAT_EXPECTED(trace_type_or_err, llvm::Succeeded());
+ASSERT_STREQ(trace_type_or_err->name.c_str(), "intel-pt");
+ASSERT_STREQ(trace_type_or_err->description.c_str(),
+ "Intel Processor Trace");
+  }
+
+  // Error response - wrong json
+  {
+std::future> result = std::async(
+std::launch::async, [&] { return client.SendGetSupportedTraceType(); });
+
+HandlePacket(server, "jLLDBTraceSupportedType", R"({"type":"intel-pt"}])");
+
+llvm::Expected trace_type_or_err = result.get();
+ASSERT_THAT_EXPECTED(
+trace_type_or_err,
+llvm::Failed(testing::Property(
+&StringError::getMessage,
+testing::HasSubstr("missing value at (root).name";
+  }
+
+  // Error response
+  {
+std::future> result = std::async(
+std::launch::async, [&] { return client.SendGetSupportedTraceType(); });
+
+HandlePacket(server, "jLLDBTraceSupportedType", "E23");
+llvm::Expected trace_type_or_err = result.get();
+ASSERT_THAT_EXPECTED(trace_type_or_err, llvm::Failed());
+  }
+
+  // Error response with error message
+  {
+std::future> result = std::async(
+std::launch::async, [&] { return client.SendGetSupportedTraceType(); });
+
+HandlePacket(server, "jLLDBTraceSupportedType",
+ "E23;50726F63657373206E6F742072756E6E696E672E");
+llvm::Expected trace_type_or_err = result.get();
+ASSERT_THAT_EXPECTED(trace_type_or_err,
+ llvm::Failed(testing::Property(
+ &StringError::getMessage,
+ testing::HasSubstr("Process not running.";
+  }
+}
+
 TEST_F(GDBRemoteCommunicationClientTest, SendStartTracePacket) {
   TraceOptions options;
   Status error;
Index: lldb/source/Utility/TraceOptions.cpp
===
--- /dev/null
+++ lldb/source/Utility/TraceOptions.cpp
@@ -0,0 +1,25 @@
+//===-- TraceOptions.cpp *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Utility/TraceOptions.h"
+
+using namespace lldb_private;
+
+namespace llvm {
+namespace json {
+
+bool fromJSON(const Value &value, TraceTypeInfo &info, Path path) {
+  ObjectMapper o(value, path);
+  if (!o)
+return false;
+  o.map(

[Lldb-commits] [lldb] b7c06dc - [ThreadPlan] Add a test for `thread step-in -r`, NFC (reapply)

[Vedant Kumar via lldb-commits]

Author: Vedant Kumar
Date: 2020-11-11T10:43:38-08:00
New Revision: b7c06dcb739f8a30c8dc9d4cc8832a9c939a1c49

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

LOG: [ThreadPlan] Add a test for `thread step-in -r`, NFC (reapply)

Adds test coverage for ThreadPlanStepInRange::SetAvoidRegexp, but
disables the test on Windows.

See:
http://lab.llvm.org:8080/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/lldb/source/Target/ThreadPlanStepInRange.cpp.html#L309

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

Added: 
lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py

Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/test/API/lang/c/stepping/main.c

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index f7d59a8a065c..89c25eb55516 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -126,6 +126,8 @@
 
 SOURCE_DISPLAYED_CORRECTLY = "Source code displayed correctly"
 
+STEP_IN_SUCCEEDED = "Thread step-in succeeded"
+
 STEP_OUT_SUCCEEDED = "Thread step-out succeeded"
 
 STOPPED_DUE_TO_EXC_BAD_ACCESS = "Process should be stopped due to bad access 
exception"

diff  --git a/lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py 
b/lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py
new file mode 100644
index ..ca58b4afcb57
--- /dev/null
+++ b/lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py
@@ -0,0 +1,34 @@
+"""
+Test thread step-in [ -r | --step-over-regexp ].
+"""
+
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class ThreadStepInAvoidRegexTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+self.line2 = line_number('main.c', '// assignment to B2')
+
+@skipIfWindows
+def test_step_out_avoid_regexp(self):
+"""Exercise thread step-in -r"""
+self.build()
+lldbutil.run_to_source_breakpoint(self,
+'frame select 2, thread step-out while stopped',
+lldb.SBFileSpec('main.c'))
+
+# Now step in, skipping the frames for 'b' and 'a'.
+self.runCmd("thread step-in -r 'a'")
+
+# We should be at the assignment to B2.
+self.expect("thread backtrace", STEP_IN_SUCCEEDED,
+substrs=["stop reason = step in"],
+patterns=["frame #0.*main.c:%d" % self.line2])

diff  --git a/lldb/test/API/lang/c/stepping/main.c 
b/lldb/test/API/lang/c/stepping/main.c
index 43b5cfab183a..8237ebf984c9 100644
--- a/lldb/test/API/lang/c/stepping/main.c
+++ b/lldb/test/API/lang/c/stepping/main.c
@@ -39,7 +39,7 @@ int main (int argc, char const *argv[])
 {
 int A1 = a(1); // frame select 2, thread step-out while stopped at "c(1)"
 
-int B2 = b(2);
+int B2 = b(2); // assignment to B2
 
 int A3 = a(3); // frame select 1, thread step-out while stopped at "c(3)"
 



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

[Lldb-commits] [lldb] fc8c1ea - [lldb/test] Add missing decorators import

[Vedant Kumar via lldb-commits]

Author: Vedant Kumar
Date: 2020-11-11T10:48:27-08:00
New Revision: fc8c1ea9aff376526e5e18f60caaf84b81785886

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

LOG: [lldb/test] Add missing decorators import

Added: 


Modified: 
lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py

Removed: 




diff  --git a/lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py 
b/lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py
index ca58b4afcb57..36588606b651 100644
--- a/lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py
+++ b/lldb/test/API/lang/c/stepping/TestThreadStepInAvoidRegexp.py
@@ -7,6 +7,7 @@
 import lldb
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
 
 
 class ThreadStepInAvoidRegexTestCase(TestBase):



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

[Lldb-commits] [PATCH] D90490: [intel-pt][trace] Implement a "get supported trace type" packet

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

It looks like this broke the windows lldb buildbot:

http://lab.llvm.org:8011/#/builders/83/builds/693


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90490

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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 304614.

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

https://reviews.llvm.org/D77153

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
  
lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  lldb/test/API/lang/c/anonymous/TestAnonymous.py

Index: lldb/test/API/lang/c/anonymous/TestAnonymous.py
===
--- lldb/test/API/lang/c/anonymous/TestAnonymous.py
+++ lldb/test/API/lang/c/anonymous/TestAnonymous.py
@@ -62,7 +62,11 @@
 
 # These should display correctly.
 self.expect("expression pz", VARIABLES_DISPLAYED_CORRECTLY,
-substrs=["(type_z *) $", " = 0x"])
+substrs=["(type_z *) $", " = 0x0"])
+self.expect("expression --format d -- pz", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=["(type_z *) $", " = 0"])
+self.expect("expression --format b -- pz", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=["(type_z *) $", " = 0b0"])
 
 self.expect("expression z.y", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["(type_y) $", "dummy = 2"])
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -74,6 +74,7 @@
 std::u32string u32_string(U"🍄🍅🍆🍌");
 std::u32string u32_empty(U"");
 std::basic_string uchar(5, 'a');
+std::string *null_str = nullptr;
 
 #if _LIBCPP_ABI_VERSION == 1
 std::string garbage1, garbage2, garbage3, garbage4, garbage5;
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -77,6 +77,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 self.runCmd("n")
@@ -114,6 +115,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 # The test assumes that std::string is in its cap-size-data layout.
Index: lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
@@ -39,4 +39,4 @@
 # Both `std::vector` and the type of the member have forward
 # declarations before their definitions.
 self.expect("expr --raw -- v",
-substrs=['(std::__1::vector) $0 = {', 'f = 0x', '}'])
+substrs=['(std::__1::vector) $0 = {', 'f = nullptr', '}'])
Index: lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
@@ -27,6 +27,8 @@
 return lldb::eLanguageTypeObjC_plus_plus;
   }
 
+  llvm::StringRef GetNilReferenceSummaryString() override { return "nil"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() const override { return &m_highlighter; }
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -119,6 +119,8 @@
 
   

[Lldb-commits] [PATCH] D90490: [intel-pt][trace] Implement a "get supported trace type" packet

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

@stella.stamenova I'm working on a fix right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90490

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

[Lldb-commits] [lldb] d2f18e6 - Fix 21555fff4de811309ea7935f9cb65578c957d77f

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2020-11-11T12:30:24-08:00
New Revision: d2f18e6b1e0bed97f5218af499c4e0b88c4dd361

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

LOG: Fix 21555fff4de811309ea7935f9cb65578c957d77f

Buildbot failed on Windows
http://lab.llvm.org:8011/#/builders/83/builds/693

Error: On Windows, std::future can't hold an Expected, as it doesn't have a 
default
constructor.

Solution: Use std::future instead of std::future>

Added: 


Modified: 
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Removed: 




diff  --git 
a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp 
b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
index d4f7b25714fb..adfead6aed98 100644
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -363,59 +363,62 @@ TEST_F(GDBRemoteCommunicationClientTest, 
GetMemoryRegionInfoInvalidResponse) {
 }
 
 TEST_F(GDBRemoteCommunicationClientTest, SendTraceSupportedTypePacket) {
+  TraceTypeInfo trace_type;
+  std::string error_message;
+  auto callback = [&] {
+if (llvm::Expected trace_type_or_err =
+client.SendGetSupportedTraceType()) {
+  trace_type = *trace_type_or_err;
+  error_message = "";
+  return true;
+} else {
+  trace_type = {};
+  error_message = llvm::toString(trace_type_or_err.takeError());
+  return false;
+}
+  };
+
   // Success response
   {
-std::future> result = std::async(
-std::launch::async, [&] { return client.SendGetSupportedTraceType(); 
});
+std::future result = std::async(std::launch::async, callback);
 
 HandlePacket(
 server, "jLLDBTraceSupportedType",
 R"({"name":"intel-pt","description":"Intel Processor Trace"}])");
 
-llvm::Expected trace_type_or_err = result.get();
-EXPECT_THAT_EXPECTED(trace_type_or_err, llvm::Succeeded());
-ASSERT_STREQ(trace_type_or_err->name.c_str(), "intel-pt");
-ASSERT_STREQ(trace_type_or_err->description.c_str(),
- "Intel Processor Trace");
+EXPECT_TRUE(result.get());
+ASSERT_STREQ(trace_type.name.c_str(), "intel-pt");
+ASSERT_STREQ(trace_type.description.c_str(), "Intel Processor Trace");
   }
 
   // Error response - wrong json
   {
-std::future> result = std::async(
-std::launch::async, [&] { return client.SendGetSupportedTraceType(); 
});
+std::future result = std::async(std::launch::async, callback);
 
 HandlePacket(server, "jLLDBTraceSupportedType", R"({"type":"intel-pt"}])");
 
-llvm::Expected trace_type_or_err = result.get();
-ASSERT_THAT_EXPECTED(
-trace_type_or_err,
-llvm::Failed(testing::Property(
-&StringError::getMessage,
-testing::HasSubstr("missing value at (root).name";
+EXPECT_FALSE(result.get());
+ASSERT_STREQ(error_message.c_str(), "missing value at (root).name");
   }
 
   // Error response
   {
-std::future> result = std::async(
-std::launch::async, [&] { return client.SendGetSupportedTraceType(); 
});
+std::future result = std::async(std::launch::async, callback);
 
 HandlePacket(server, "jLLDBTraceSupportedType", "E23");
-llvm::Expected trace_type_or_err = result.get();
-ASSERT_THAT_EXPECTED(trace_type_or_err, llvm::Failed());
+
+EXPECT_FALSE(result.get());
   }
 
   // Error response with error message
   {
-std::future> result = std::async(
-std::launch::async, [&] { return client.SendGetSupportedTraceType(); 
});
+std::future result = std::async(std::launch::async, callback);
 
 HandlePacket(server, "jLLDBTraceSupportedType",
  "E23;50726F63657373206E6F742072756E6E696E672E");
-llvm::Expected trace_type_or_err = result.get();
-ASSERT_THAT_EXPECTED(trace_type_or_err,
- llvm::Failed(testing::Property(
- &StringError::getMessage,
- testing::HasSubstr("Process not running.";
+
+EXPECT_FALSE(result.get());
+ASSERT_STREQ(error_message.c_str(), "Process not running.");
   }
 }
 



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

[Lldb-commits] [PATCH] D91293: [lldb] [Process/FreeBSDRemote] Modernize and simplify YMM logic

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

mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski.
mgorny requested review of this revision.

Eliminate the remaining swith-case code for register getters,
and migrate YMM registers to regset-oriented model.  Since these
registers are recombined from XMM and YMM_Hi128 XSAVE blocks, while LLDB
gdb-server protocol transmits YMM registers whole, the offset-based
model will not work here.  Nevertheless, some improvement was possible.

Replace generic 'XSaveRegSet' along with sub-sets for XSAVE components
with 'YMMRegSet' (and more regsets in the future as further components
are implemented).  Create a helper GetYMMSplitReg() method that obtains
pointers to the appropriate XMM and YMM_Hi128 blocks to reduce code
duplication.


https://reviews.llvm.org/D91293

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
@@ -58,15 +58,13 @@
 private:
   // Private member types.
   enum RegSetKind {
+YMMRegSet,
+MaxXSaveSet = YMMRegSet,
+
 GPRegSet,
 FPRegSet,
-XSaveRegSet,
 DBRegSet,
   };
-  enum {
-YMMXSaveSet,
-MaxXSaveSet = YMMXSaveSet,
-  };
 
   // Private member variables.
   std::array m_gpr;
@@ -82,6 +80,7 @@
 
   size_t GetFPROffset() const;
   size_t GetDBROffset() const;
+  bool GetYMMSplitReg(uint32_t reg, void **xmm, void **ymm_hi);
 };
 
 } // namespace process_freebsd
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -275,31 +275,6 @@
   }
 }
 
-static constexpr int RegNumX86ToX86_64(int regnum) {
-  switch (regnum) {
-  case lldb_ymm0_i386:
-  case lldb_ymm1_i386:
-  case lldb_ymm2_i386:
-  case lldb_ymm3_i386:
-  case lldb_ymm4_i386:
-  case lldb_ymm5_i386:
-  case lldb_ymm6_i386:
-  case lldb_ymm7_i386:
-return lldb_ymm0_x86_64 + regnum - lldb_ymm0_i386;
-  case lldb_bnd0_i386:
-  case lldb_bnd1_i386:
-  case lldb_bnd2_i386:
-  case lldb_bnd3_i386:
-return lldb_bnd0_x86_64 + regnum - lldb_bnd0_i386;
-  case lldb_bndcfgu_i386:
-return lldb_bndcfgu_x86_64;
-  case lldb_bndstatus_i386:
-return lldb_bndstatus_x86_64;
-  default:
-llvm_unreachable("Unhandled i386 register.");
-  }
-}
-
 llvm::Optional
 NativeRegisterContextFreeBSD_x86_64::GetSetForNativeRegNum(int reg_num) const {
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
@@ -309,7 +284,7 @@
 if (reg_num >= k_first_fpr_i386 && reg_num <= k_last_fpr_i386)
   return FPRegSet;
 if (reg_num >= k_first_avx_i386 && reg_num <= k_last_avx_i386)
-  return XSaveRegSet; // AVX
+  return YMMRegSet;
 if (reg_num >= k_first_mpxr_i386 && reg_num <= k_last_mpxr_i386)
   return llvm::None; // MPXR
 if (reg_num >= k_first_mpxc_i386 && reg_num <= k_last_mpxc_i386)
@@ -323,7 +298,7 @@
 if (reg_num >= k_first_fpr_x86_64 && reg_num <= k_last_fpr_x86_64)
   return FPRegSet;
 if (reg_num >= k_first_avx_x86_64 && reg_num <= k_last_avx_x86_64)
-  return XSaveRegSet; // AVX
+  return YMMRegSet;
 if (reg_num >= k_first_mpxr_x86_64 && reg_num <= k_last_mpxr_x86_64)
   return llvm::None; // MPXR
 if (reg_num >= k_first_mpxc_x86_64 && reg_num <= k_last_mpxc_x86_64)
@@ -354,7 +329,7 @@
   case DBRegSet:
 return NativeProcessFreeBSD::PtraceWrapper(PT_GETDBREGS, m_thread.GetID(),
m_dbr.data());
-  case XSaveRegSet: {
+  case YMMRegSet: {
 struct ptrace_xstate_info info;
 Status ret = NativeProcessFreeBSD::PtraceWrapper(
 PT_GETXSTATE_INFO, GetProcessPid(), &info, sizeof(info));
@@ -364,10 +339,10 @@
 assert(info.xsave_mask & XFEATURE_ENABLED_X87);
 assert(info.xsave_mask & XFEATURE_ENABLED_SSE);
 
-m_xsave_offsets[YMMXSaveSet] = LLDB_INVALID_XSAVE_OFFSET;
+m_xsave_offsets[YMMRegSet] = LLDB_INVALID_XSAVE_OFFSET;
 if (info.xsave_mask & XFEATURE_ENABLED_YMM_HI128) {
   uint32_t eax, ecx, edx;
-  __get_cpuid_count(0x0D, 2, &eax, &m_xsave_offsets[YMMXSaveSet], &ecx,
+  __get_cpuid_count(0x0D, 2, &eax, &m_xsave_offsets[YMMRegSet], &ecx,
 &edx);
 }
 
@@ -395,7 +370,7 @@
   case DBRegSet:
 return NativeProcessFreeBSD::PtraceWrapper(PT_SETDBREGS, m_thread.GetID(),
m_dbr.data());
-  case XSaveRegSet:
+  case YMMRegSet:
 // Re

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a subscriber: sgraenitz.
JDevlieghere added a comment.

In D69589#1769663 , @sgraenitz wrote:

> I'd recommend to be careful when making a change of this size. There are a 
> lot of possible configurations.
> If you split it up into a few smaller and more dedicated commits, you will 
> make your life easier. Otherwise someone might just revert everything if this 
> happens to break a bot.

+1

Please include me as a reviewer when making changes like this to the build 
system, especially if it affects the framework build. It's only sparsely tested 
upstream (in part because we have a bunch of downstream changes) but it is the 
way we ship LLDB to our customers so it is really import to us. If I happen to 
miss the review I likely won't discover issues until the next time we rebranch 
at which point it might be a lot harder to figure them out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69589

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

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Ha, I looked at the date but didn't realize this was a review from fall last 
year :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69589

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

[Lldb-commits] [PATCH] D91238: Recognize __builtin_debugtrap on arm64, advance pc past it so users can continue easily

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Another possible way to do this would be to make a new stop reason in LLDB. 
debugserver currently can return a "reason" whose values are one of "trap", 
"breakpoint", "watchpoint", "exception", or "exec". What if we added 
"debug-trap"? Then back in LLDB we could add "eStopReasonDebugTrap" to 
"lldb::StopReason". Then the ABI plug-in or some arch specific plug-in could do 
something in response to this on stop, or on continue. x86 would know that is 
doesn't need to do anything because the PC is already past the trap 
instructions. But ARM64 it could advance the PC when the process goes to start 
running again so it skips the trap instruction. I would presonally like to see 
the program tell the truth when you hit a "brk #0xf000" and show the PC being 
on this instruction, and then continue from it correctly when you resume. Your 
solution is much easier though, so either way works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91238

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

[Lldb-commits] [PATCH] D91130: [crashlog] Implement parser for JSON encoded crashlogs

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D91130#2387692 , @JDevlieghere 
wrote:

> In D91130#2387390 , @clayborg wrote:
>
>> JSON crash logs are almost here??? I remember asking for those back when I 
>> was at Apple! Great news.
>
> Yep! 🥳

Nice!

>> It would be nice to have a crash log from a small but real process we might 
>> be able to load as an end to end test?
>
> I'd be happy to, but is there a way to test the crashlog script without 
> having a `dsymForUUID`?

Yes, it would still load it up correctly and just show you the stack frame that 
are in the JSON format. The only reason you need dsymForUUID is to expand 
inline frames and add source file and line info if it isn't already in the 
crash log. So the test would test that it can parse the JSON format correctly 
and display what is already in the crash log without needing to actually 
re-symbolicate. "crashlog" doesn't assume it will be able to find symbols for 
all images. It will keep the string from the crashlog file that describes the 
address:

  Thread 0:: CrBrowserMain  Dispatch queue: com.apple.main-thread
  0   libsystem_kernel.dylib0x7fff678a2dfa mach_msg_trap + 10
  1   libsystem_kernel.dylib0x7fff678a3170 mach_msg + 60
  2   com.apple.CoreFoundation  0x7fff2d6f4ef5 
__CFRunLoopServiceMachPort + 247
  3   com.apple.CoreFoundation  0x7fff2d6f39c2 __CFRunLoopRun + 1319
  4   com.apple.CoreFoundation  0x7fff2d6f2e3e CFRunLoopRunSpecific 
+ 462
  5   com.apple.HIToolbox   0x7fff2c31fabd 
RunCurrentEventLoopInMode + 292
  6   com.apple.HIToolbox   0x7fff2c31f7d5 
ReceiveNextEventCommon + 584
  7   com.apple.HIToolbox   0x7fff2c31f579 
_BlockUntilNextEventMatchingListInModeWithFilter + 64
  8   com.apple.AppKit  0x7fff2a965039 _DPSNextEvent + 883
  9   com.apple.AppKit  0x7fff2a963880 
-[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] 
+ 1352
  10  com.apple.AppKit  0x7fff2a95558e -[NSApplication run] 
+ 658
  11  com.apple.AppKit  0x7fff2a927396 NSApplicationMain + 
777
  12  com.sketchup.SketchUp.20170x00010a62638c main + 144
  13  libdyld.dylib 0x7fff67761cc9 start + 1

So for the first frame here from the text based crash log it would keep 
"mach_msg_trap + 10" in case it can't symbolicate any better.


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

https://reviews.llvm.org/D91130

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

[Lldb-commits] [PATCH] D90729: [trace][intel-pt] Scaffold the 'thread trace start | stop' commands

[walter erquinigo via Phabricator via lldb-commits]

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

Address comments:

- Don't cache the command delegate. It was overcomplicating the code for little 
gain.
- Create a RecordedProcess base class for trace and coredump processes, so that 
we can put the IsLiveDebugSession method there, and in the future we can add 
more common methods there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90729

Files:
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/Interpreter/CommandObjectDelegate.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/ProcessTrace.h
  lldb/include/lldb/Target/RecordedProcess.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectThreadUtil.cpp
  lldb/source/Commands/CommandObjectThreadUtil.h
  lldb/source/Core/PluginManager.cpp
  lldb/source/Interpreter/CMakeLists.txt
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Interpreter/CommandObjectDelegate.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.h
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/source/Target/Process.cpp
  lldb/source/Target/ProcessTrace.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/TestTraceStartStop.py

Index: lldb/test/API/commands/trace/TestTraceStartStop.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceStartStop.py
@@ -0,0 +1,73 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceLoad(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def setUp(self):
+TestBase.setUp(self)
+if 'intel-pt' not in configuration.enabled_plugins:
+self.skipTest("The intel-pt test plugin is not enabled")
+
+def expectGenericHelpMessageForStartCommand(self):
+self.expect("help thread trace start",
+substrs=["Syntax: thread trace start []"])
+
+def testStartStopSessionFileThreads(self):
+# it should fail for processes from json session files
+self.expect("trace load -v " + os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"))
+self.expect("thread trace start", error=True,
+substrs=["error: tracing is not supported. Can't trace a non-live process"])
+
+# the help command should be the generic one, as it's not a live process
+self.expectGenericHelpMessageForStartCommand()
+
+# this should fail because 'trace stop' is not yet implemented
+self.expect("thread trace stop", error=True,
+substrs=["error: failed stopping thread 3842849"])
+
+@skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
+def testStartStopLiveThreads(self):
+# The help command should be the generic one if there's no process running
+self.expectGenericHelpMessageForStartCommand()
+
+self.expect("thread trace start", error=True,
+substrs=["error: invalid target, create a target using the 'target create' command"])
+
+self.expect("file " + os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+self.expect("b main")
+
+self.expect("thread trace start", error=True,
+substrs=["error: invalid process"])
+
+# The help command should be the generic one if there's still no process running
+self.expectGenericHelpMessageForStartCommand()
+
+self.expect("r")
+
+# the help command should be the intel-pt one now
+self.expect("help thread trace start",
+substrs=["Start tracing one or more threads with intel-pt.",
+ "Syntax: thread trace start [  ...] []"])
+
+self.expect("thread trace start",
+patterns=["would trace tid .* with size_in_kb 4 and custom_config 0"])
+
+self.expect("thread trace start --size 20 --custom-config 1",
+patterns=["would trace tid .* with size_in_kb 20 and custom_config 1"])
+
+# This fails because "trace stop" is not yet implemented.
+self.exp

[Lldb-commits] [PATCH] D91130: [crashlog] Implement parser for JSON encoded crashlogs

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

You could also make a fakey dsymForUUID script like we do in some other 
corefile tests.  Compile a C file with foo(), bar(), baz(), substitute the 
addresses of those symbols from the binary into the crashlog.json, substitute 
the UUID of the compiled program in the crashlog.json, then the 
fakey-dsymForUUID finds the binary, lldb loads it at the binary at the correct 
offset.  It's a lot of futzing around, but it would be possible.


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

https://reviews.llvm.org/D91130

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

[Lldb-commits] [PATCH] D90729: [trace][intel-pt] Scaffold the 'thread trace start | stop' commands

[walter erquinigo via Phabricator via lldb-commits]

wallace marked 10 inline comments as done.
wallace added inline comments.



Comment at: lldb/include/lldb/Target/ProcessTrace.h:1
 //===-- ProcessTrace.h --*- C++ 
-*-===//
 //

clayborg wrote:
> Should ProcessTrace and ThreadTrace be moved to the Process plug-in directory?
can't do that. I think Pavel and other guys warned me of cross-referencing 
between plugin-folders. It is safer this way, as they are base classes for 
trace plug-ins.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90729

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

[Lldb-commits] [PATCH] D91130: [crashlog] Implement parser for JSON encoded crashlogs

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D91130#2389831 , @jasonmolenda 
wrote:

> You could also make a fakey dsymForUUID script like we do in some other 
> corefile tests.  Compile a C file with foo(), bar(), baz(), substitute the 
> addresses of those symbols from the binary into the crashlog.json, substitute 
> the UUID of the compiled program in the crashlog.json, then the 
> fakey-dsymForUUID finds the binary, lldb loads it at the binary at the 
> correct offset.  It's a lot of futzing around, but it would be possible.

Crash logs can be loaded on other machines as well. Nothing stopping the test 
from running on linux, windows, or other systems. The main thing we want to 
test is if the JSON format creates the right objects with the right info


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

https://reviews.llvm.org/D91130

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

[Lldb-commits] [PATCH] D91238: Recognize __builtin_debugtrap on arm64, advance pc past it so users can continue easily

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

Yeah, I don't know where it's best to centralize the logic of (1) recognize a 
builtin_debugtrap instruction, and (2) set the pc so we step past it, know how 
to step past it when the user asks to resume control.  debugserver (and 
lldb-server I'm sure) are already lying a *little* with a normal x86 user 
breakpoint - when you execute 0xcc at addr 0x100, the stop pc is 0x101 and 
someone is rolling that back to 0x100 to report the breakpoint hit (debugserver 
does this, I'm sure lldb-server does the same).  So we're just adding more lies 
into the remote stub.

You could argue that the stub should always set $pc to be the instruction that 
hit the breakpoint/undefined instruction (so on x86, back up the pc by 1) and 
lldb should look at the breakpoint list / instruction bytes and decide if this 
was a user breakpoint or a builtin_trap of some kind.  And when it's a 
builtin_debugtrap, and the user continues/steps/nexts, lldb could advance $pc 
past the instruction and then do the nexting.

I don't have much of an opinion about where this knowledge and pc adjusting 
should happen.  Jim might though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91238

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

[Lldb-commits] [PATCH] D91238: Recognize __builtin_debugtrap on arm64, advance pc past it so users can continue easily

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D91238#2389877 , @jasonmolenda 
wrote:

> Yeah, I don't know where it's best to centralize the logic of (1) recognize a 
> builtin_debugtrap instruction, and (2) set the pc so we step past it, know 
> how to step past it when the user asks to resume control.  debugserver (and 
> lldb-server I'm sure) are already lying a *little* with a normal x86 user 
> breakpoint - when you execute 0xcc at addr 0x100, the stop pc is 0x101 and 
> someone is rolling that back to 0x100 to report the breakpoint hit 
> (debugserver does this, I'm sure lldb-server does the same).  So we're just 
> adding more lies into the remote stub.
>
> You could argue that the stub should always set $pc to be the instruction 
> that hit the breakpoint/undefined instruction (so on x86, back up the pc by 
> 1) and lldb should look at the breakpoint list / instruction bytes and decide 
> if this was a user breakpoint or a builtin_trap of some kind.  And when it's 
> a builtin_debugtrap, and the user continues/steps/nexts, lldb could advance 
> $pc past the instruction and then do the nexting.
>
> I don't have much of an opinion about where this knowledge and pc adjusting 
> should happen.  Jim might though.

No worries, this solution is simple and will work just fine. I have no issues 
either way. It will be interesting to see what Jim suggests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91238

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

[Lldb-commits] [PATCH] D91130: [crashlog] Implement parser for JSON encoded crashlogs

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D91130#2389867 , @clayborg wrote:

> 



> Crash logs can be loaded on other machines as well. Nothing stopping the test 
> from running on linux, windows, or other systems. The main thing we want to 
> test is if the JSON format creates the right objects with the right info

Yeah, fakey-dsymForUUID isn't necessary.  You compile a userland C program, 
grab the addresses of a few of its functions, substitute them & the UUID into 
the crashlog.json, then create an SBTarget with the compiled program and run 
the crashlog.json and test that it slid the binary & prints the right backtrace 
or whatever.


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

https://reviews.llvm.org/D91130

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

[Lldb-commits] [PATCH] D91130: [crashlog] Implement parser for JSON encoded crashlogs

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

I guess the register context part of the crashlog.json would be the most 
architecture-dependent, but I bet crashlog.json doesn't use any of that when 
printing the backtrace -- it only uses the stack trace's pc values.  So you 
could have a "x86_64" crashlog.json, build an arm64e userland program, 
substitute in the slide-adjusted values of a few functions and the UUID and it 
would load & symbolicate in lldb.


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

https://reviews.llvm.org/D91130

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

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

[Haibo Huang via Phabricator via lldb-commits]

hhb closed this revision.
hhb added a comment.

I gave up on this. 😊


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69589

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

[Lldb-commits] [PATCH] D90729: [trace][intel-pt] Scaffold the 'thread trace start | stop' commands

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/include/lldb/Interpreter/CommandObjectDelegate.h:30-32
+  virtual lldb::CommandObjectSP DoGetDelegateCommand() = 0;
+
+  CommandObject *GetDelegateCommand();

So currently you are storing the command object into m_delegate_command_sp in 
GetDelegateCommand(). This can cause an issue when you have two debuggers 
(Xcode creates a new debugger for each debug window). If thread A calls this 
function, and fills in m_delegate_command_sp, then thread B fills in 
m_delegate_command_sp from another command interpreter, we could end up using 
the one from thread B. Since the only reason we are storing the value in 
m_delegate_command_sp is to keep the shared pointer alive, we should just 
return the shared pointer. So I would suggest removing DoGetDelegateCommand() 
and just have:

```
virtual llvm::Expected GetDelegateCommand() = 0;
```
Don't store it. Each function that calls it will keep it around as long as 
needed. If we use expected then we don't need to store the m_delegate_error 
later...



Comment at: lldb/source/Interpreter/CommandObjectDelegate.cpp:16-19
+  m_delegate_command_sp = DoGetDelegateCommand();
+  if (m_delegate_command_sp)
+return m_delegate_command_sp.get();
+  return nullptr;

This caching isn't really doing anything. If we have two debuggers, each one 
has its own command interpreter and this can actually cause things to fail. If 
thread A calls this, then thread B, then thread A continues to use the one 
populated by thread B. See inline comment in header file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90729

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

[Lldb-commits] [PATCH] D91238: Recognize __builtin_debugtrap on arm64, advance pc past it so users can continue easily

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

The attraction of having stub fix up the PC in the stop after hitting the trap 
for breakpoints (in this case by moving the PC before the trap on architectures 
that execute the trap) was that this decision could be made simply in the stub, 
but if lldb had to check DECR_PC_AFTER_BREAK to understand a breakpoint stop, 
that would need to happen in a bunch of different places (as it did in gdb) and 
would be easier to get wrong.

But the only job we would need to do to proceed past unrecognized traps is to 
check, when we are about to continue, whether the next instruction is a trap.  
If it is a trap we would push a "step over trap" plan before whatever else we 
were going to do.  And in fact, we already do that for breakpoints because the 
user might have put a breakpoint on the next instruction before hitting 
"continue" and we really don't want to stop at that breakpoint.  We always 
check this last thing before the continue.  So implementing proceed past 
unrecognized trap in lldb would be very contained.  We'd just have to expand 
the "is there a breakpoint at the current PC" to "is there a breakpoint or any 
other architecture defined 'trap' instruction at the current PC."

That weakens the argument for doing it in the stub rather than in lldb.

Doing it in lldb is a little nicer too, since on systems that execute their 
traps, you would just never see a trap instruction at the PC when you were 
about to continue, so the same solution would work.  And as Pavel says, then we 
wouldn't have to change lldb-server as well.

So provided lldb has an easy way to answer the question "is this a trap I see 
before me" I'm mildly in favor of extending lldb's "should I push a 
step-over-breakpoint plan now" logic to check both "is there a breakpoint here" 
and "is there any other architecture defined trap here".  But not enough  to 
say you shouldn't do it this way...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91238

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

[Lldb-commits] [PATCH] D90729: [trace][intel-pt] Scaffold the 'thread trace start | stop' commands

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Walter and I discussed some issues offline and came to the conclusion we need 
to use CommandObjectProxy and get rid of CommandObjectDelegate. This will 
simplify the patch and use the pre-existing class that already does what we 
needed CommandObjectDelegate to do...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90729

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

[Lldb-commits] [lldb] 0783ad9 - [lldb] Switch expect to runCmd in TestRecursiveTypes (NFC)

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2020-11-11T16:17:38-08:00
New Revision: 0783ad9e6a2c1156ca0bf5493473d6c98f006593

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

LOG: [lldb] Switch expect to runCmd in TestRecursiveTypes (NFC)

Following discussion in D91193, a change made in D88792 was not quite right.
This restores the message argument, and switches from `expect` to `runCmd`.

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

Added: 


Modified: 
lldb/test/API/types/TestRecursiveTypes.py

Removed: 




diff  --git a/lldb/test/API/types/TestRecursiveTypes.py 
b/lldb/test/API/types/TestRecursiveTypes.py
index 8e84a052a22b..8deb193677a9 100644
--- a/lldb/test/API/types/TestRecursiveTypes.py
+++ b/lldb/test/API/types/TestRecursiveTypes.py
@@ -50,5 +50,5 @@ def print_struct(self):
 
 self.runCmd("run", RUN_SUCCEEDED)
 
-self.expect("print tpi")
-self.expect("print *tpi")
+self.runCmd("print tpi")
+self.runCmd("print *tpi")



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

[Lldb-commits] [PATCH] D91206: [lldb] Switch expect to runCmd in TestRecursiveTypes (NFC)

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0783ad9e6a2c: [lldb] Switch expect to runCmd in 
TestRecursiveTypes (NFC) (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91206

Files:
  lldb/test/API/types/TestRecursiveTypes.py


Index: lldb/test/API/types/TestRecursiveTypes.py
===
--- lldb/test/API/types/TestRecursiveTypes.py
+++ lldb/test/API/types/TestRecursiveTypes.py
@@ -50,5 +50,5 @@
 
 self.runCmd("run", RUN_SUCCEEDED)
 
-self.expect("print tpi")
-self.expect("print *tpi")
+self.runCmd("print tpi")
+self.runCmd("print *tpi")


Index: lldb/test/API/types/TestRecursiveTypes.py
===
--- lldb/test/API/types/TestRecursiveTypes.py
+++ lldb/test/API/types/TestRecursiveTypes.py
@@ -50,5 +50,5 @@
 
 self.runCmd("run", RUN_SUCCEEDED)
 
-self.expect("print tpi")
-self.expect("print *tpi")
+self.runCmd("print tpi")
+self.runCmd("print *tpi")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91130: [crashlog] Implement parser for JSON encoded crashlogs

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 304685.
JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.

Add tests for JSON and text format


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

https://reviews.llvm.org/D91130

Files:
  lldb/examples/python/crashlog.py
  lldb/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/Assertion.check
  lldb/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json_parser.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/Assertion.check
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.crash
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/test.c
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/crashlog.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/parser_json.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/parser_text.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/patch-crashlog.py
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test

Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
@@ -0,0 +1,10 @@
+# RUN: %clang_host -g %S/Inputs/test.c -o %t.out
+# RUN: cp %S/Inputs/a.out.crash %t.crash
+# RUN: python %S/patch-crashlog.py %t.out %t.crash '{"main":20, "bar":9, "foo":16}'
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
+
+# CHECK: Thread[0] EXC_BAD_ACCESS (SIGSEGV) (KERN_INVALID_ADDRESS at 0x)
+# CHECK: [  0] {{.*}}out`foo + 16 at test.c
+# CHECK: [  1] {{.*}}out`bar + 8 at test.c
+# CHECK: [  2] {{.*}}out`main + 19 at test.c
+# CHECK: [  3] {{.*}}start + 1
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/patch-crashlog.py
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/patch-crashlog.py
@@ -0,0 +1,60 @@
+#!/usr/bin/env python
+
+import json
+import os
+import re
+import subprocess
+import sys
+
+
+class CrashLogPatcher:
+
+SYMBOL_REGEX = re.compile(r'^([0-9a-fA-F]+) T _(.*)$')
+UUID_REGEX = re.compile(r'UUID: ([-0-9a-fA-F]+) \(([^\(]+)\) .*')
+
+def __init__(self, data, binary, offsets):
+self.data = data
+self.binary = binary
+self.offsets = offsets
+
+def patch_executable(self):
+self.data = self.data.replace("@EXEC@", self.binary)
+self.data = self.data.replace("@NAME@", os.path.basename(self.binary))
+
+def patch_uuid(self):
+output = subprocess.check_output(['dwarfdump', '--uuid', self.binary])
+m = self.UUID_REGEX.match(output)
+if m:
+self.data = self.data.replace("@UUID@", m.group(1))
+
+def patch_addresses(self):
+if not self.offsets:
+return
+output = subprocess.check_output(['nm', self.binary])
+for line in output.splitlines():
+m = self.SYMBOL_REGEX.match(line)
+if m:
+address = m.group(1)
+symbol = m.group(2)
+if symbol in self.offsets:
+patch_addr = int(m.group(1), 16) + int(
+self.offsets[symbol])
+self.data = self.data.replace("@{}@".format(symbol),
+  str(hex(patch_addr)))
+
+
+if __name__ == '__main__':
+binary = sys.argv[1]
+crashlog = sys.argv[2]
+offsets = json.loads(sys.argv[3]) if len(sys.argv) > 3 else None
+
+with open(crashlog, 'r') as file:
+data = file.read()
+
+p = CrashLogPatcher(data, binary, offsets)
+p.patch_executable()
+p.patch_uuid()
+p.patch_addresses()
+
+with open(crashlog, 'w') as file:
+file.write(p.data)
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/parser_text.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/parser_text.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/parser_text.test
@@ -4,7 +4,7 @@
 # CHECK-LABEL: {{S}}KIP BEYOND CHECKS
 script
 import crashlog
-crash_log_parser = crashlog.CrashLogParser
+crash_log_parser = crashlog.TextCrashLogParser
 crash_log = crashlog.CrashLog
 images = [
 "0x10b60b000 - 0x10f707fff com.apple.LLDB.framework (1.1000.11.38.2 - 1000.11.38.2) <96E36F5C-1A83-39A1-8713-5FDD9701C3F1> /Applications/Xcode.app/Contents/SharedFrameworks/LLDB.framework/LLDB",
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/parser_json.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/parser_json.test
@@ -0,0 +1,45 @@
+#  

[Lldb-commits] [PATCH] D90729: [trace][intel-pt] Scaffold the 'thread trace start | stop' commands

[walter erquinigo via Phabricator via lldb-commits]

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

- Use CommandObjectProxy and delete the Delegate that I created
- I had to add a few methods to the proxy to match with that I need. In any 
case, the implementation is not more complete.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90729

Files:
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/Interpreter/CommandObjectMultiword.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/ProcessTrace.h
  lldb/include/lldb/Target/RecordedProcess.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectMultiword.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectThreadUtil.cpp
  lldb/source/Commands/CommandObjectThreadUtil.h
  lldb/source/Core/PluginManager.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.h
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/source/Target/Process.cpp
  lldb/source/Target/ProcessTrace.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/TestTraceStartStop.py

Index: lldb/test/API/commands/trace/TestTraceStartStop.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceStartStop.py
@@ -0,0 +1,73 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceLoad(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def setUp(self):
+TestBase.setUp(self)
+if 'intel-pt' not in configuration.enabled_plugins:
+self.skipTest("The intel-pt test plugin is not enabled")
+
+def expectGenericHelpMessageForStartCommand(self):
+self.expect("help thread trace start",
+substrs=["Syntax: thread trace start []"])
+
+def testStartStopSessionFileThreads(self):
+# it should fail for processes from json session files
+self.expect("trace load -v " + os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"))
+self.expect("thread trace start", error=True,
+substrs=["error: tracing is not supported. Can't trace a non-live process"])
+
+# the help command should be the generic one, as it's not a live process
+self.expectGenericHelpMessageForStartCommand()
+
+# this should fail because 'trace stop' is not yet implemented
+self.expect("thread trace stop", error=True,
+substrs=["error: failed stopping thread 3842849"])
+
+@skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
+def testStartStopLiveThreads(self):
+# The help command should be the generic one if there's no process running
+self.expectGenericHelpMessageForStartCommand()
+
+self.expect("thread trace start", error=True,
+substrs=["error: invalid target, create a target using the 'target create' command"])
+
+self.expect("file " + os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+self.expect("b main")
+
+self.expect("thread trace start", error=True,
+substrs=["error: invalid process"])
+
+# The help command should be the generic one if there's still no process running
+self.expectGenericHelpMessageForStartCommand()
+
+self.expect("r")
+
+# the help command should be the intel-pt one now
+self.expect("help thread trace start",
+substrs=["Start tracing one or more threads with intel-pt.",
+ "Syntax: thread trace start [  ...] []"])
+
+self.expect("thread trace start",
+patterns=["would trace tid .* with size_in_kb 4 and custom_config 0"])
+
+self.expect("thread trace start --size 20 --custom-config 1",
+patterns=["would trace tid .* with size_in_kb 20 and custom_config 1"])
+
+# This fails because "trace stop" is not yet implemented.
+self.expect("thread trace stop", error=True,
+substrs=["error: Process is not being traced"])
+
+self.expect("c")
+# Now the p

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.

It seems weird that even if you had a summary formatter for some pointer type 
that was trying to print "DANGER WILL ROBINSON" when the pointer value was 0x0, 
we will override that and print "nullptr" in a C++ context or "nil" in an ObjC 
context.  Seems like we even if the value is Nil we should first consult the 
ValueObject's summary value and only do the nullptr/nil computation if it was 
empty.

But that bad behavior was already present for ObjC objects before this patch, 
you're just extending it to C++ or anything else that implements 
IsNilReference.  So fixing that doesn't seem required for this patch.




Comment at: lldb/source/DataFormatters/ValueObjectPrinter.cpp:368
+  summary.assign(lang_plugin->GetNilReferenceSummaryString().str());
+  } else if (IsUninitialized()) {
+summary.assign("");

If you wanted to you could check here if the languages hadn't assigned anything 
to Summary, and default to "NULL"?  That would be consistent with our treating 
C as the fallback language rather than as a separate Language plugin.  Not 
required...


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

https://reviews.llvm.org/D77153

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

[Lldb-commits] [lldb] 856fd98 - Generalize regex matching std::string variants to compensate for recent

[Richard Smith via lldb-commits]

Author: Richard Smith
Date: 2020-11-11T17:55:47-08:00
New Revision: 856fd98a176240470dcc2b8ad54b5c17ef6a75b3

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

LOG: Generalize regex matching std::string variants to compensate for recent
improvements to Clang's type printing.

Added: 


Modified: 
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 83a140ebd5e7..7bc5b194514a 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -461,52 +461,52 @@ static void LoadLibCxxFormatters(lldb::TypeCategoryImplSP 
cpp_category_sp) {
   AddCXXSummary(cpp_category_sp,
 lldb_private::formatters::LibcxxStringSummaryProviderASCII,
 "std::string summary provider",
-ConstString("^std::__[[:alnum:]]+::string$"), 
stl_summary_flags,
+ConstString("^std::(__[[:alnum:]]+::)?string$"), 
stl_summary_flags,
 true);
   AddCXXSummary(cpp_category_sp,
 lldb_private::formatters::LibcxxStringSummaryProviderASCII,
 "std::string summary provider",
-ConstString("^std::__[[:alnum:]]+::basic_string, "
-"std::__[[:alnum:]]+::allocator >$"),
+ConstString("^std::(__[[:alnum:]]+::)?basic_string, "
+"std::(__[[:alnum:]]+::)?allocator )?>$"),
 stl_summary_flags, true);
   AddCXXSummary(cpp_category_sp,
 lldb_private::formatters::LibcxxStringSummaryProviderASCII,
 "std::string summary provider",
-ConstString("^std::__[[:alnum:]]+::basic_string, "
-"std::__[[:alnum:]]+::allocator 
>$"),
+ConstString("^std::(__[[:alnum:]]+::)?basic_string, "
+"std::(__[[:alnum:]]+::)?allocator 
)?>$"),
 stl_summary_flags, true);
 
   AddCXXSummary(cpp_category_sp,
 lldb_private::formatters::LibcxxStringSummaryProviderUTF16,
 "std::u16string summary provider",
 ConstString(
-"^std::__[[:alnum:]]+::basic_string, "
-"std::__[[:alnum:]]+::allocator >$"),
+"^std::(__[[:alnum:]]+::)?basic_string, "
+"std::(__[[:alnum:]]+::)?allocator )?>$"),
 stl_summary_flags, true);
 
   AddCXXSummary(cpp_category_sp,
 lldb_private::formatters::LibcxxStringSummaryProviderUTF32,
 "std::u32string summary provider",
 ConstString(
-"^std::__[[:alnum:]]+::basic_string, "
-"std::__[[:alnum:]]+::allocator >$"),
+"^std::(__[[:alnum:]]+::)?basic_string, "
+"std::(__[[:alnum:]]+::)?allocator )?>$"),
 stl_summary_flags, true);
 
   AddCXXSummary(cpp_category_sp,
 lldb_private::formatters::LibcxxWStringSummaryProvider,
 "std::wstring summary provider",
-ConstString("^std::__[[:alnum:]]+::wstring$"),
+ConstString("^std::(__[[:alnum:]]+::)?wstring$"),
 stl_summary_flags, true);
   AddCXXSummary(cpp_category_sp,
 lldb_private::formatters::LibcxxWStringSummaryProvider,
 "std::wstring summary provider",
-ConstString("^std::__[[:alnum:]]+::basic_string, "
-"std::__[[:alnum:]]+::allocator >$"),
+ConstString("^std::(__[[:alnum:]]+::)?basic_string, "
+"std::(__[[:alnum:]]+::)?allocator )?>$"),
 stl_summary_flags, true);
 
   SyntheticChildren::Flags stl_synth_flags;
@@ -787,6 +787,10 @@ static void 
LoadLibStdcppFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   ConstString("std::__cxx11::basic_string, "
   "std::allocator >"),
   cxx11_string_summary_sp);
+  cpp_category_sp->GetTypeSummariesContainer()->Add(
+  ConstString("std::basic_string, "
+  "std::allocator >"),
+  cxx11_string_summary_sp);
   cpp_category_sp->GetTypeSummariesContainer()->Add(
   ConstString("std::__cxx11::basic_string, "
   "std::allocator >"),



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

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

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 304726.
omjavaid added a comment.

This update addresses comments about last revision. Also sending offset field 
in qRegisterInfo and XML register description has been disabled D91241 



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

https://reviews.llvm.org/D82863

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1767,6 +1767,19 @@
 gdb_thread->PrivateSetRegisterValue(pair.first, buffer_sp->GetData());
   }
 
+  // AArch64 SVE specific code below calls AArch64SVEReconfigure to update
+  // SVE register sizes and offsets if value of VG register has changed
+  // since last stop.
+  const ArchSpec &arch = GetTarget().GetArchitecture();
+  if (arch.IsValid() && arch.GetTriple().isAArch64()) {
+GDBRemoteRegisterContext *reg_ctx_sp =
+static_cast(
+gdb_thread->GetRegisterContext().get());
+
+if (reg_ctx_sp)
+  reg_ctx_sp->AArch64SVEReconfigure();
+  }
+
   thread_sp->SetName(thread_name.empty() ? nullptr : thread_name.c_str());
 
   gdb_thread->SetThreadDispatchQAddr(thread_dispatch_qaddr);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -40,6 +40,9 @@
 
   void HardcodeARMRegisters(bool from_scratch);
 
+  bool UpdateARM64SVERegistersInfos(uint64_t vg, uint32_t &end_reg_offset,
+std::vector &invalidate_regs);
+
   void CloneFrom(GDBRemoteDynamicRegisterInfoSP process_reginfo);
 };
 
@@ -79,6 +82,8 @@
   uint32_t ConvertRegisterKindToRegisterNumber(lldb::RegisterKind kind,
uint32_t num) override;
 
+  bool AArch64SVEReconfigure();
+
 protected:
   friend class ThreadGDBRemote;
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -213,8 +213,8 @@
   for (int i = 0; i < regcount; i++) {
 struct RegisterInfo *reginfo =
 m_reg_info_sp->GetRegisterInfoAtIndex(i);
-if (reginfo->byte_offset + reginfo->byte_size 
-   <= buffer_sp->GetByteSize()) {
+if (reginfo->byte_offset + reginfo->byte_size <=
+buffer_sp->GetByteSize()) {
   m_reg_valid[i] = true;
 } else {
   m_reg_valid[i] = false;
@@ -343,6 +343,15 @@
   if (dst == nullptr)
 return false;
 
+  // Code below is specific to AArch64 target in SVE state
+  // If vector granule (vg) register is being written then thread's
+  // register context reconfiguration is triggered on success.
+  bool do_reconfigure_arm64_sve = false;
+  const ArchSpec &arch = process->GetTarget().GetArchitecture();
+  if (arch.IsValid() && arch.GetTriple().isAArch64())
+if (strcmp(reg_info->name, "vg") == 0)
+  do_reconfigure_arm64_sve = true;
+
   if (data.CopyByteOrderedData(data_offset,// src offset
reg_info->byte_size,// src length
dst,// dst
@@ -362,6 +371,11 @@
 
 {
   SetAllRegisterValid(false);
+
+  if (do_reconfigure_arm64_sve &&
+  GetPrimordialRegister(reg_info, gdb_comm))
+AArch64SVEReconfigure();
+
   return true;
 }
   } else {
@@ -390,6 +404,10 @@
 } else {
   // This is an actual register, write it
   success = SetPrimordialRegister(reg_info, gdb_comm);
+
+  if (success && do_reconfigure_arm64_sve &&
+  GetPrimordialRegister(reg_info, gdb_comm))
+AArch64SVEReconfigure();
 }
 
 // Check if writing this register will invalidate any other register
@@ -655,7 +673,7 @@
   if (m_thread.GetProcess().get()) {
 const ArchSpec &arch =
 m_thread.GetProcess()->GetTarget().GetArchitecture();
-if (arch.IsValid() && 
+if (ar

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

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 304728.
omjavaid added a comment.

Minor update after changes to D82863 .


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

https://reviews.llvm.org/D82857

Files:
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h

Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
@@ -14,6 +14,8 @@
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/StructuredData.h"
 
+#include "GDBRemoteRegisterContext.h"
+
 class StringExtractor;
 
 namespace lldb_private {
@@ -101,6 +103,10 @@
   m_queue_serial_number; // Queue info from stop reply/stop info for thread
   lldb_private::LazyBool m_associated_with_libdispatch_queue;
 
+  GDBRemoteDynamicRegisterInfoSP m_reg_info_sp;
+
+  void SetThreadRegisterInfo();
+
   bool PrivateSetRegisterValue(uint32_t reg, llvm::ArrayRef data);
 
   bool PrivateSetRegisterValue(uint32_t reg, uint64_t regval);
Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
@@ -42,6 +42,9 @@
   Log *log(GetLogIfAnyCategoriesSet(GDBR_LOG_THREAD));
   LLDB_LOG(log, "this = {0}, pid = {1}, tid = {2}", this, process.GetID(),
GetID());
+  // At this point we can clone reg_info for architectures supporting
+  // run-time update to register sizes and offsets..
+  SetThreadRegisterInfo();
 }
 
 ThreadGDBRemote::~ThreadGDBRemote() {
@@ -307,8 +310,8 @@
   !pSupported || gdb_process->m_use_g_packet_for_reading;
   bool write_all_registers_at_once = !pSupported;
   reg_ctx_sp = std::make_shared(
-  *this, concrete_frame_idx, gdb_process->m_register_info,
-  read_all_registers_at_once, write_all_registers_at_once);
+  *this, concrete_frame_idx, m_reg_info_sp, read_all_registers_at_once,
+  write_all_registers_at_once);
 }
   } else {
 reg_ctx_sp = GetUnwinder().CreateRegisterContextForFrame(frame);
@@ -316,6 +319,23 @@
   return reg_ctx_sp;
 }
 
+void ThreadGDBRemote::SetThreadRegisterInfo() {
+  ProcessSP process_sp(GetProcess());
+  if (process_sp) {
+ProcessGDBRemote *gdb_process =
+static_cast(process_sp.get());
+
+if (!m_reg_info_sp) {
+  if (!gdb_process->m_register_info_sp->IsReconfigurable())
+m_reg_info_sp = gdb_process->m_register_info_sp;
+  else {
+m_reg_info_sp = std::make_shared();
+m_reg_info_sp->CloneFrom(gdb_process->m_register_info_sp);
+  }
+}
+  }
+}
+
 bool ThreadGDBRemote::PrivateSetRegisterValue(uint32_t reg,
   llvm::ArrayRef data) {
   GDBRemoteRegisterContext *gdb_reg_ctx =
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -253,7 +253,7 @@
  // the last stop
  // packet variable
   std::recursive_mutex m_last_stop_packet_mutex;
-  GDBRemoteDynamicRegisterInfo m_register_info;
+  GDBRemoteDynamicRegisterInfoSP m_register_info_sp;
   Broadcaster m_async_broadcaster;
   lldb::ListenerSP m_async_listener_sp;
   HostThread m_async_thread;
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -248,7 +248,7 @@
ListenerSP listener_sp)
 : Process(target_sp, listener_sp),
   m_debugserver_pid(LLDB_INVALID_PROCESS_ID), m_last_stop_packet_mutex(),
-  m_register_info(),
+  m_register_info_sp(nullptr),
   m_async_broadcaster(nullptr, "lldb.process.gdb-remote.async-broadcaster"),
   m_async_listener_sp(
   Listener::MakeListener("lldb.process.gdb-remote.async-listener")),
@@ -367,8 +367,8 @@
   m_breakpoint_pc_offset = breakpoint_pc_int_value->GetValue();
   }
 
-  if (m_register_info.Set