[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

2023-04-12 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 513047.
JDevlieghere marked 4 inline comments as done.
JDevlieghere added a comment.

Address @mib's code review feedback


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

https://reviews.llvm.org/D148172

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/symbolication.py
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test

Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
@@ -41,3 +41,6 @@
 # CHECK-NEXT:frame #2: 0x000100ec5a87 multithread-test`compute_pow{{.*}} [artificial]
 # CHECK:frame #{{[0-9]+}}: 0x00019cc7e06b{{.*}} [artificial]
 # CHECK:frame #{{[0-9]+}}: 0x00019cc78e2b{{.*}} [artificial]
+
+image list
+# CHECK: ---- {{.*}}bogus.dylib
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
@@ -478,6 +478,15 @@
   "source" : "A",
   "base" : 0,
   "uuid" : "----"
+},
+{
+  "arch": "arm64",
+  "base": 12345,
+  "name": "bogus.dylib",
+  "path": "/usr/lib/system/bogus.dylib",
+  "size": 1000,
+  "source": "P",
+  "uuid": "----"
 }
   ],
   "userID": 501,
Index: lldb/examples/python/symbolication.py
===
--- lldb/examples/python/symbolication.py
+++ lldb/examples/python/symbolication.py
@@ -35,6 +35,8 @@
 import sys
 import time
 import uuid
+import json
+import tempfile
 
 
 class Address:
@@ -230,6 +232,7 @@
 def __init__(self, path, uuid=None):
 self.path = path
 self.resolved_path = None
+self.resolve = False
 self.resolved = False
 self.unavailable = False
 self.uuid = uuid
@@ -240,6 +243,7 @@
 self.module = None
 self.symfile = None
 self.slide = None
+self.symbols = dict()
 
 @classmethod
 def InitWithSBTargetAndSBModule(cls, target, module):
@@ -372,14 +376,33 @@
 uuid_str = self.get_normalized_uuid_string()
 if uuid_str:
 self.module = target.AddModule(None, None, uuid_str)
-if not self.module:
+if not self.module and self.resolve:
 self.locate_module_and_debug_symbols()
-if self.unavailable:
-return None
-resolved_path = self.get_resolved_path()
-self.module = target.AddModule(
-resolved_path, None, uuid_str, self.symfile)
-if not self.module:
+if not self.unavailable:
+resolved_path = self.get_resolved_path()
+self.module = target.AddModule(
+resolved_path, None, uuid_str, self.symfile)
+if not self.module and self.section_infos:
+name = os.path.basename(self.path)
+with tempfile.NamedTemporaryFile(suffix='.' + name) as tf:
+data = {
+'triple': target.triple,
+'uuid': uuid_str,
+'type': 'sharedlibrary',
+'sections': list(),
+'symbols': list()
+}
+for section in self.section_infos:
+data['sections'].append({
+'name' : section.name,
+'size': section.end_addr - section.start_addr
+})
+for symbol in self.symbols.values():
+data['symbols'].append(symbol)
+with open(tf.name, 'w') as f:
+f.write(json.dumps(data, indent=4))
+self.module = target.AddModule(tf.name, None, uuid_str)
+if not self.module and not self.unavailable:
 return 'error: unable to get module for (%s) "%s"' % (
 self.arch, self.get_resolved_path())
 if self.has_section_load_info():
Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

2023-04-12 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere marked 7 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/examples/python/crashlog.py:460-464
+def __init__(self, debugger, path, verbose):
+super().__init__(debugger, path, verbose)
+# List of DarwinImages sorted by their index.
+self.images = list()
+

mib wrote:
> Why do we need this ?
The parser now stores a list of images sorted by their index. We can't use the 
list of images in the crashlog (`self.crashlog.images`) because they have the 
main module at index `0`.



Comment at: lldb/examples/python/crashlog.py:515
 def parse_images(self, json_images):
-idx = 0
-for json_image in json_images:
+for idx, json_image in enumerate(json_images):
 img_uuid = uuid.UUID(json_image['uuid'])

mib wrote:
> What do we use `idx` for ?
You're right, this isn't necessary anymore.



Comment at: lldb/examples/python/crashlog.py:526-527
  self.verbose)
+self.images.append(darwin_image)
 self.crashlog.images.append(darwin_image)
 

mib wrote:
> Seems like we're doing the same things for both `self.images` and 
> `self.crashlog.images` ... In that case, is `self.images` really necessary ?
Yes, see my previous comment. 



Comment at: lldb/examples/python/symbolication.py:402-403
+})
+for symbol in self.symbols.values():
+data['symbols'].append(symbol)
+with open(tf.name, 'w') as f:

mib wrote:
> This should work, right ?
Apparently not: `values()` returns a view object rather than a list: 
https://docs.python.org/3/library/stdtypes.html#dict-views


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

https://reviews.llvm.org/D148172

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

[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

2023-04-12 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.

LGTM! Thanks


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

https://reviews.llvm.org/D148062

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

[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

2023-04-12 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 513045.
JDevlieghere marked 4 inline comments as done.
JDevlieghere added a comment.

Address @mib's code review feedback


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

https://reviews.llvm.org/D148062

Files:
  lldb/include/lldb/Core/Section.h
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/source/Core/Section.cpp
  lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
  lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.h
  lldb/source/Symbol/ObjectFile.cpp
  lldb/test/API/macosx/symbols/TestObjectFileJSON.py

Index: lldb/test/API/macosx/symbols/TestObjectFileJSON.py
===
--- /dev/null
+++ lldb/test/API/macosx/symbols/TestObjectFileJSON.py
@@ -0,0 +1,100 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+import json
+import uuid
+import os
+import shutil
+
+
+class TestObjectFileJSON(TestBase):
+TRIPLE = "arm64-apple-macosx13.0.0"
+
+def setUp(self):
+TestBase.setUp(self)
+self.source = "main.c"
+
+def emitJSON(self, data, path):
+json_object = json.dumps(data, indent=4)
+json_object_file = self.getBuildArtifact("a.json")
+with open(json_object_file, "w") as outfile:
+outfile.write(json_object)
+
+def toModuleSpec(self, path):
+module_spec = lldb.SBModuleSpec()
+module_spec.SetFileSpec(lldb.SBFileSpec(path))
+return module_spec
+
+@no_debug_info_test
+def test_target(self):
+data = {
+"triple": self.TRIPLE,
+"uuid": str(uuid.uuid4()),
+"type": "executable",
+}
+
+json_object_file = self.getBuildArtifact("a.json")
+self.emitJSON(data, json_object_file)
+
+target = self.dbg.CreateTarget(json_object_file)
+self.assertTrue(target.IsValid())
+self.assertEqual(target.GetTriple(), self.TRIPLE)
+
+@no_debug_info_test
+def test_module(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+target = self.dbg.CreateTarget(exe)
+
+data = {
+"triple": self.TRIPLE,
+"uuid": str(uuid.uuid4()),
+}
+
+json_object_file = self.getBuildArtifact("a.json")
+self.emitJSON(data, json_object_file)
+
+module = target.AddModule(self.toModuleSpec(json_object_file))
+self.assertFalse(module.IsValid())
+
+data = {
+"triple": self.TRIPLE,
+"uuid": str(uuid.uuid4()),
+"type": "sharedlibrary",
+"sections": [
+{
+"name": "__TEXT",
+"type": "code",
+"address": 0,
+"size": 0x222,
+}
+],
+"symbols": [
+{
+"name": "foo",
+"address": 0x100,
+"size": 0x11,
+}
+],
+}
+self.emitJSON(data, json_object_file)
+
+module = target.AddModule(self.toModuleSpec(json_object_file))
+self.assertTrue(module.IsValid())
+
+section = module.GetSectionAtIndex(0)
+self.assertTrue(section.IsValid())
+self.assertEqual(section.GetName(), "__TEXT")
+self.assertEqual(section.file_addr, 0x0)
+self.assertEqual(section.size, 0x222)
+
+symbol = module.FindSymbol("foo")
+self.assertTrue(symbol.IsValid())
+self.assertEqual(symbol.addr.GetFileAddress(), 0x100)
+self.assertEqual(symbol.GetSize(), 0x11)
+
+error = target.SetSectionLoadAddress(section, 0x1000)
+self.assertSuccess(error)
+self.assertEqual(symbol.addr.GetLoadAddress(target), 0x1100)
Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -761,3 +761,34 @@
   m_cache_hash = llvm::djbHash(strm.GetString());
   return *m_cache_hash;
 }
+
+namespace llvm {
+namespace json {
+
+bool fromJSON(const llvm::json::Value ,
+  lldb_private::ObjectFile::Type , llvm::json::Path path) {
+  if (auto str = value.getAsString()) {
+type = llvm::StringSwitch(*str)
+   .Case("corefile", ObjectFile::eTypeCoreFile)
+   .Case("executable", ObjectFile::eTypeExecutable)
+   .Case("debuginfo", ObjectFile::eTypeDebugInfo)
+   .Case("dynamiclinker", ObjectFile::eTypeDynamicLinker)
+   .Case("objectfile", ObjectFile::eTypeObjectFile)
+   .Case("sharedlibrary", ObjectFile::eTypeSharedLibrary)
+   .Case("stublibrary", ObjectFile::eTypeStubLibrary)
+   .Case("jit", ObjectFile::eTypeJIT)
+   .Case("unknown", ObjectFile::eTypeUnknown)
+

[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

2023-04-12 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Core/Section.cpp:683-684
+  llvm::json::ObjectMapper o(value, path);
+  return o && o.map("name", section.name) && o.map("type", section.type) &&
+ o.map("size", section.address) && o.map("size", section.size);
+}

mib wrote:
> We should use `o.mapOptional` here since the `address`, `size` and `type` are 
> marked optional.
They're not exactly the same: `map` with a `std::optional` lets you deal with 
whether the value was set outside of the deserialization method, while 
`mapOptional` requires you to have initialized the variable to the default 
value previously, or pass it as a third argument. In this particular case it 
doesn't really matter, as the default value is pretty trivial, but I'd rather 
preserve that a value wasn't set. That's different from the call to 
`mapOptional` for the symbol and section vectors, where I don't care whether 
the vector is empty or was omitted completely. 


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

https://reviews.llvm.org/D148062

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

[Lldb-commits] [PATCH] D132608: [CMake] Clean up CMake binary dir handling

2023-04-12 Thread Fangrui Song via Phabricator via lldb-commits 
MaskRay added a comment.
Herald added subscribers: bviyer, ekilmer, jplehr, thopre.

@Ericson2314  @phosek What's the state of this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132608

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

[Lldb-commits] [lldb] e76cfac - AArch64 debugserver parse ESR register for watchpoints

2023-04-12 Thread Jason Molenda via lldb-commits 

Author: Jason Molenda
Date: 2023-04-12T18:36:17-07:00
New Revision: e76cfaca70be0b45e62149e52f68d8352fa8ea2f

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

LOG: AArch64 debugserver parse ESR register for watchpoints

Have debugserver parse the watchpoint flags out of the exception
syndrome register when we get a watchpoint mach exception.  Relay
those fields up to lldb in the stop reply packet, if the watchpoint
number was reported by the hardware, use the address from that as
the watchpoint address.

Change how watchpoints are reported to lldb from using the mach
exception data, to using the `reason:watchpoint` and `description:asciihex`
method that lldb-server uses, which can relay the actual trap address
as well as the address of a watched memory region responsible for
the trap, so lldb can step past it.

Have debugserver look for the nearest watchpoint that it has set
when it gets a watchpoint trap, so accesses that are reported as
starting before the watched region are associated with the correct
watchpoint to lldb.  Add a test case for this specific issue.

Differential Revision: https://reviews.llvm.org/D147820
rdar://83996471

Added: 
lldb/test/API/commands/watchpoints/unaligned-watchpoint/Makefile

lldb/test/API/commands/watchpoints/unaligned-watchpoint/TestUnalignedWatchpoint.py
lldb/test/API/commands/watchpoints/unaligned-watchpoint/main.c

Modified: 
lldb/tools/debugserver/source/DNBBreakpoint.cpp
lldb/tools/debugserver/source/DNBBreakpoint.h
lldb/tools/debugserver/source/DNBDefs.h
lldb/tools/debugserver/source/MacOSX/MachException.cpp
lldb/tools/debugserver/source/MacOSX/MachProcess.h
lldb/tools/debugserver/source/MacOSX/MachProcess.mm
lldb/tools/debugserver/source/RNBRemote.cpp

Removed: 




diff  --git a/lldb/test/API/commands/watchpoints/unaligned-watchpoint/Makefile 
b/lldb/test/API/commands/watchpoints/unaligned-watchpoint/Makefile
new file mode 100644
index 0..10495940055b6
--- /dev/null
+++ b/lldb/test/API/commands/watchpoints/unaligned-watchpoint/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/commands/watchpoints/unaligned-watchpoint/TestUnalignedWatchpoint.py
 
b/lldb/test/API/commands/watchpoints/unaligned-watchpoint/TestUnalignedWatchpoint.py
new file mode 100644
index 0..de1a0511633c7
--- /dev/null
+++ 
b/lldb/test/API/commands/watchpoints/unaligned-watchpoint/TestUnalignedWatchpoint.py
@@ -0,0 +1,55 @@
+"""
+Watch one byte in the middle of a doubleword, mutate the
+entire doubleword including the watched byte.  On AArch64
+the trap address is probably the start of the doubleword,
+instead of the address of our watched byte.  Test that lldb
+correctly associates this watchpoint trap with our watchpoint.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class UnalignedWatchpointTestCase(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+@skipIfOutOfTreeDebugserver
+def test_unaligned_watchpoint(self):
+"""Test an unaligned watchpoint triggered by a larger aligned write."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.c")
+(target, process, thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
+ "break here", self.main_source_file)
+
+frame = thread.GetFrameAtIndex(0)
+
+self.expect("watchpoint set variable a.buf[2]")
+
+self.runCmd("process continue")
+
+# We should be stopped again due to the watchpoint (write type), but
+# only once.  The stop reason of the thread should be watchpoint.
+self.expect("thread list", STOPPED_DUE_TO_WATCHPOINT,
+substrs=['stopped',
+ 'stop reason = watchpoint'])
+
+# Use the '-v' option to do verbose listing of the watchpoint.
+# The hit count should now be 1.
+self.expect("watchpoint list -v",
+substrs=['hit_count = 1'])
+
+self.runCmd("process continue")
+
+# We should be stopped again due to the watchpoint (write type), but
+# only once.  The stop reason of the thread should be watchpoint.
+self.expect("thread list", STOPPED_DUE_TO_WATCHPOINT,
+substrs=['stopped',
+ 'stop reason = watchpoint'])
+
+# Use the '-v' option to do verbose listing of the watchpoint.
+# The hit count should now be 1.
+self.expect("watchpoint list -v",
+substrs=['hit_count = 2'])

diff  --git a/lldb/test/API/commands/watchpoints/unaligned-watchpoint/main.c

[Lldb-commits] [PATCH] D147820: debugserver: move AArch64 watchpoint traps within a watchpointed region, parse ESR flags and send them to lldb

2023-04-12 Thread Jason Molenda via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe76cfaca70be: AArch64 debugserver parse ESR register for 
watchpoints (authored by jasonmolenda).

Changed prior to commit:
  https://reviews.llvm.org/D147820?vs=512577=513036#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147820

Files:
  lldb/test/API/commands/watchpoints/unaligned-watchpoint/Makefile
  
lldb/test/API/commands/watchpoints/unaligned-watchpoint/TestUnalignedWatchpoint.py
  lldb/test/API/commands/watchpoints/unaligned-watchpoint/main.c
  lldb/tools/debugserver/source/DNBBreakpoint.cpp
  lldb/tools/debugserver/source/DNBBreakpoint.h
  lldb/tools/debugserver/source/DNBDefs.h
  lldb/tools/debugserver/source/MacOSX/MachException.cpp
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -2841,11 +2841,21 @@
   InitializeRegisters();
 
 if (g_reg_entries != NULL) {
+  auto interesting_regset = [](int regset) -> bool {
+#if defined(__arm64__) || defined(__aarch64__)
+// GPRs and exception registers, helpful for debugging
+// from packet logs.
+return regset == 1 || regset == 3;
+#else
+return regset == 1;
+#endif
+  };
+
   DNBRegisterValue reg_value;
   for (uint32_t reg = 0; reg < g_num_reg_entries; reg++) {
 // Expedite all registers in the first register set that aren't
 // contained in other registers
-if (g_reg_entries[reg].nub_info.set == 1 &&
+if (interesting_regset(g_reg_entries[reg].nub_info.set) &&
 g_reg_entries[reg].nub_info.value_regs == NULL) {
   if (!DNBThreadGetRegisterValueByID(
   pid, tid, g_reg_entries[reg].nub_info.set,
@@ -2860,6 +2870,51 @@
 
 if (did_exec) {
   ostrm << "reason:exec;";
+} else if (tid_stop_info.reason == eStopTypeWatchpoint) {
+  ostrm << "reason:watchpoint;";
+  ostrm << "description:";
+  std::ostringstream wp_desc;
+  wp_desc << tid_stop_info.details.watchpoint.addr << " ";
+  wp_desc << tid_stop_info.details.watchpoint.hw_idx << " ";
+  wp_desc << tid_stop_info.details.watchpoint.mach_exception_addr;
+  append_hexified_string(ostrm, wp_desc.str());
+  ostrm << ";";
+
+  // Temporarily, print all of the fields we've parsed out of the ESR
+  // on a watchpoint exception.  Normally this is something we would
+  // log for LOG_WATCHPOINTS only, but this was implemented from the
+  // ARM ARM spec and hasn't been exercised on real hardware that can
+  // set most of these fields yet.  It may need to be debugged in the
+  // future, so include all of these purely for debugging by reading
+  // the packet logs; lldb isn't using these fields.
+  ostrm << "watch_addr:" << std::hex
+<< tid_stop_info.details.watchpoint.addr << ";";
+  ostrm << "me_watch_addr:" << std::hex
+<< tid_stop_info.details.watchpoint.mach_exception_addr << ";";
+  ostrm << "wp_hw_idx:" << std::hex
+<< tid_stop_info.details.watchpoint.hw_idx << ";";
+  if (tid_stop_info.details.watchpoint.esr_fields_set) {
+ostrm << "wp_esr_iss:" << std::hex
+  << tid_stop_info.details.watchpoint.esr_fields.iss << ";";
+ostrm << "wp_esr_wpt:" << std::hex
+  << tid_stop_info.details.watchpoint.esr_fields.wpt << ";";
+ostrm << "wp_esr_wptv:"
+  << tid_stop_info.details.watchpoint.esr_fields.wptv << ";";
+ostrm << "wp_esr_wpf:"
+  << tid_stop_info.details.watchpoint.esr_fields.wpf << ";";
+ostrm << "wp_esr_fnp:"
+  << tid_stop_info.details.watchpoint.esr_fields.fnp << ";";
+ostrm << "wp_esr_vncr:"
+  << tid_stop_info.details.watchpoint.esr_fields.vncr << ";";
+ostrm << "wp_esr_fnv:"
+  << tid_stop_info.details.watchpoint.esr_fields.fnv << ";";
+ostrm << "wp_esr_cm:" << tid_stop_info.details.watchpoint.esr_fields.cm
+  << ";";
+ostrm << "wp_esr_wnr:"
+  << tid_stop_info.details.watchpoint.esr_fields.wnr << ";";
+ostrm << "wp_esr_dfsc:" << std::hex
+  << tid_stop_info.details.watchpoint.esr_fields.dfsc << ";";
+  }
 } else if (tid_stop_info.details.exception.type) {
   ostrm << "metype:" << std::hex << tid_stop_info.details.exception.type
 << ';';
@@ -5475,6 +5530,20 @@
   }
   break;
 
+case eStopTypeWatchpoint: {
+  reason_value = "watchpoint";
+  thread_dict_sp->AddIntegerItem("watchpoint",
+

[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

2023-04-12 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib added inline comments.



Comment at: lldb/lldb/test/API/macosx/symbols/TestObjectFileJSON.py:12
+
+class TestObjectFileJSOn(TestBase):
+TRIPLE = "arm64-apple-macosx13.0.0"

nit



Comment at: lldb/source/Core/Section.cpp:683-684
+  llvm::json::ObjectMapper o(value, path);
+  return o && o.map("name", section.name) && o.map("type", section.type) &&
+ o.map("size", section.address) && o.map("size", section.size);
+}

We should use `o.mapOptional` here since the `address`, `size` and `type` are 
marked optional.



Comment at: lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp:111
 
+  if (data_sp->GetByteSize() < length) {
+data_sp = MapFileData(file, length, file_offset);

I guess this is due to the fact that we only read the first 512 bytes from the 
binary the first time we load it ? It would be good to mention it in a comment.



Comment at: lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp:210
+  return o && o.map("triple", header.triple) && o.map("uuid", header.uuid) &&
+ o.map("type", header.type);
 }

ditto



Comment at: lldb/test/API/macosx/symbols/TestObjectFileJSON.py:12
+
+class TestObjectFileJSOn(TestBase):
+TRIPLE = "arm64-apple-macosx13.0.0"

It looks like you've included this test twice.


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

https://reviews.llvm.org/D148062

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

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

2023-04-12 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib added inline comments.



Comment at: lldb/examples/python/symbolication.py:399
+'name' : section.name,
+'address': 0,
+'size': section.end_addr - section.start_addr

Since this field is optional in the ObjectFileJSON, I think it should be 
included here. We should just initialize it to `0` in lldb if the user didn't 
provide it.


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

https://reviews.llvm.org/D148172

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

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

2023-04-12 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib added inline comments.



Comment at: lldb/examples/python/crashlog.py:460-464
+def __init__(self, debugger, path, verbose):
+super().__init__(debugger, path, verbose)
+# List of DarwinImages sorted by their index.
+self.images = list()
+

Why do we need this ?



Comment at: lldb/examples/python/crashlog.py:515
 def parse_images(self, json_images):
-idx = 0
-for json_image in json_images:
+for idx, json_image in enumerate(json_images):
 img_uuid = uuid.UUID(json_image['uuid'])

What do we use `idx` for ?



Comment at: lldb/examples/python/crashlog.py:526-527
  self.verbose)
+self.images.append(darwin_image)
 self.crashlog.images.append(darwin_image)
 

Seems like we're doing the same things for both `self.images` and 
`self.crashlog.images` ... In that case, is `self.images` really necessary ?



Comment at: 
lldb/examples/python/scripted_process/crashlog_scripted_process.py:25-33
+if self.load_all_images:
+for image in self.crashlog.images:
+image.resolve = True
+else:
+for thread in self.crashlog.threads:
+if thread.did_crash():
+for ident in thread.idents:

Nice!



Comment at: lldb/examples/python/symbolication.py:387
+if not self.module and self.section_infos:
+name = pathlib.Path(self.path).name
+with tempfile.NamedTemporaryFile(suffix='.' + name) as tf:

Fancy! Can't we just do this and remove the extra import ?



Comment at: lldb/examples/python/symbolication.py:402-403
+})
+for symbol in self.symbols.values():
+data['symbols'].append(symbol)
+with open(tf.name, 'w') as f:

This should work, right ?


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

https://reviews.llvm.org/D148172

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

[Lldb-commits] [PATCH] D147816: Clarify how watchpoint description in stop packets work, fix AArch64 unintended behavior

2023-04-12 Thread Jason Molenda via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe38b0fa83a93: Remove AArch64 out of MIPS watchpoint-skip, 
doc wp description (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147816

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Target/StopInfo.h
  lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/StopInfo.cpp

Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -666,9 +666,8 @@
 WatchpointSP watchpoint_sp;
   };
 
-  StopInfoWatchpoint(Thread , break_id_t watch_id,
- lldb::addr_t watch_hit_addr)
-  : StopInfo(thread, watch_id), m_watch_hit_addr(watch_hit_addr) {}
+  StopInfoWatchpoint(Thread , break_id_t watch_id, bool silently_skip_wp)
+  : StopInfo(thread, watch_id), m_silently_skip_wp(silently_skip_wp) {}
 
   ~StopInfoWatchpoint() override = default;
 
@@ -893,27 +892,9 @@
 
 WatchpointSentry sentry(process_sp, wp_sp);
 
-/*
- * MIPS: Last 3bits of the watchpoint address are masked by the kernel.
- * For example:
- * 'n' is at 0x120010d00 and 'm' is 0x120010d04. When a watchpoint is
- * set at 'm', then
- * watch exception is generated even when 'n' is read/written. To handle
- * this case,
- * server emulates the instruction at PC and finds the base address of
- * the load/store
- * instruction and appends it in the description of the stop-info
- * packet. If watchpoint
- * is not set on this address by user then this do not stop.
-*/
-if (m_watch_hit_addr != LLDB_INVALID_ADDRESS) {
-  WatchpointSP wp_hit_sp =
-  thread_sp->CalculateTarget()->GetWatchpointList().FindByAddress(
-  m_watch_hit_addr);
-  if (!wp_hit_sp) {
-m_should_stop = false;
-wp_sp->IncrementFalseAlarmsAndReviseHitCount();
-  }
+if (m_silently_skip_wp) {
+  m_should_stop = false;
+  wp_sp->IncrementFalseAlarmsAndReviseHitCount();
 }
 
 if (wp_sp->GetHitCount() <= wp_sp->GetIgnoreCount()) {
@@ -1035,7 +1016,17 @@
   
   bool m_should_stop = false;
   bool m_should_stop_is_valid = false;
-  lldb::addr_t m_watch_hit_addr;
+  // A false watchpoint hit has happened -
+  // the thread stopped with a watchpoint
+  // hit notification, but the watched region
+  // was not actually accessed (as determined
+  // by the gdb stub we're talking to).
+  // Continue past this watchpoint without
+  // notifying the user; on some targets this
+  // may mean disable wp, instruction step,
+  // re-enable wp, continue.
+  // On others, just continue.
+  bool m_silently_skip_wp = false;
   bool m_step_over_plan_complete = false;
   bool m_using_step_over_plan = false;
 };
@@ -1372,10 +1363,11 @@
   return StopInfoSP(new StopInfoBreakpoint(thread, break_id, should_stop));
 }
 
-StopInfoSP
-StopInfo::CreateStopReasonWithWatchpointID(Thread , break_id_t watch_id,
-   lldb::addr_t watch_hit_addr) {
-  return StopInfoSP(new StopInfoWatchpoint(thread, watch_id, watch_hit_addr));
+StopInfoSP StopInfo::CreateStopReasonWithWatchpointID(Thread ,
+  break_id_t watch_id,
+  bool silently_continue) {
+  return StopInfoSP(
+  new StopInfoWatchpoint(thread, watch_id, silently_continue));
 }
 
 StopInfoSP StopInfo::CreateStopReasonWithSignal(Thread , int signo,
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
@@ -1749,33 +1749,60 @@
 } else if (reason == "trap") {
   // Let the trap just use the standard signal stop reason below...
 } else if (reason == "watchpoint") {
+  // We will have between 1 and 3 fields in the description.
+  //
+  // \a wp_addr which is the original start address that
+  // lldb requested be watched, or an address that the
+  // hardware reported.  This address should be within the
+  // range of a currently active watchpoint region - lldb
+  // should be able to find a watchpoint with this address.
+  //
+  // \a wp_index is the hardware watchpoint register number.
+  //
+  // \a wp_hit_addr is the actual address reported by the hardware,
+  // which may be outside the range of a region we are watching.
+

[Lldb-commits] [lldb] e38b0fa - Remove AArch64 out of MIPS watchpoint-skip, doc wp description

2023-04-12 Thread Jason Molenda via lldb-commits 

Author: Jason Molenda
Date: 2023-04-12T17:57:21-07:00
New Revision: e38b0fa83a933a6b20064a37b8d8ebd9ec1de499

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

LOG: Remove AArch64 out of MIPS watchpoint-skip, doc wp description

Watchpoints from lldb-server are sent in the stop info packet
as a `reason:watchpoint` and `description:asciihex` keys; the
latter's asciihex has one to three integer values.  This patch
documents the purpose of those three different numbers, and
clarifies the behavior on MIPS with the third number which is
outside the range of any watched memory range means to silently
skip the watchpoint.

lldb was previously using this silently skip watchpoint behavior
for AArch64 as well, but in the case of AArch64 we see a watchpoint
address outside of a watched memory range when the write BEGINS
before the watched memory range, but extends in to it.  We don't
want to silently skip these.

Differential Revision: https://reviews.llvm.org/D147816
rdar://83996471

Added: 


Modified: 
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Target/StopInfo.h
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Target/StopInfo.cpp

Removed: 




diff  --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index 4930eb1ccb712..570e70f9e54a9 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -1559,17 +1559,57 @@ for this region.
 //  "signal" stopped due to an actual unix signal, not
 //  just the debugger using a unix signal to keep
 //  the GDB remote client happy.
-//  "watchpoint". Should be used in conjunction with
-//  the "watch"/"rwatch"/"awatch" key value pairs.
+//  "watchpoint". Can be used with of the 
+//  "watch"/"rwatch"/"awatch" key value pairs.
+//  Or can be used *instead* of those keys, 
+//  with the specially formatted "description" 
field.
 //  "exception" an exception stop reason. Use with
 //  the "description" key/value pair to describe 
the
 //  exceptional event the user should see as the 
stop
 //  reason.
 //  "description" ascii-hex An ASCII hex string that contains a more 
descriptive
-//  reason that the thread stopped. This is only needed
-//  if none of the key/value pairs are enough to
-//  describe why something stopped.
-//
+//   reason that the thread stopped. This is only needed
+//   if none of the key/value pairs are enough to
+//   describe why something stopped.
+//
+//   For "reason:watchpoint", "description" is an ascii-hex
+//   encoded string with between one and three base10 numbers,
+//   space separated.  The three numbers are
+// 1. watchpoint address.  This address should always be within
+//a memory region lldb has a watchpoint on.  
+//On architectures where the actual reported hit address may
+//be outside the watchpoint that was triggered, the remote
+//stub should determine which watchpoint was triggered and
+//report an address from within its range.
+// 2. watchpoint hardware register index number.
+// 3. actual watchpoint trap address, which may be outside
+//the range of any watched region of memory. On MIPS, an addr
+//outside a watched range means lldb should disable the wp, 
+//step, re-enable the wp and continue silently.
+//
+//   On MIPS, the low 3 bits are masked so if a watchpoint is on 
+//   0x1004, a 2-byte write to 0x1000 will trigger the watchpoint 
+//   (a false positive hit), and lldb needs to disable the 
+//   watchpoint at 0x1004, inst-step, then re-enable the watchpoint
+//   and not make this a user visible event. The description here 
+//   would be "0x1004 0 0x1000". lldb needs a known watchpoint address
+//   in the first field, so it can disable it & step.
+//
+//   On AArch64 we have a related issue, where you watch 4 bytes at 
+//   0x1004, an instruction does an 8-byte write starting at 
+//   0x1000 (a true watchpoint hit) and the hardware may report the 
+//   trap address as 0x1000 - before the watched memory region - 
+//

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

2023-04-12 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 513027.
JDevlieghere added a comment.

Add missing `CHECK` line in `skipped_status_interactive_crashlog.test`.


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

https://reviews.llvm.org/D148172

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/symbolication.py
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test

Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
@@ -41,3 +41,6 @@
 # CHECK-NEXT:frame #2: 0x000100ec5a87 multithread-test`compute_pow{{.*}} [artificial]
 # CHECK:frame #{{[0-9]+}}: 0x00019cc7e06b{{.*}} [artificial]
 # CHECK:frame #{{[0-9]+}}: 0x00019cc78e2b{{.*}} [artificial]
+
+image list
+# CHECK: ---- {{.*}}bogus.dylib
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
@@ -478,6 +478,15 @@
   "source" : "A",
   "base" : 0,
   "uuid" : "----"
+},
+{
+  "arch": "arm64",
+  "base": 12345,
+  "name": "bogus.dylib",
+  "path": "/usr/lib/system/bogus.dylib",
+  "size": 1000,
+  "source": "P",
+  "uuid": "----"
 }
   ],
   "userID": 501,
Index: lldb/examples/python/symbolication.py
===
--- lldb/examples/python/symbolication.py
+++ lldb/examples/python/symbolication.py
@@ -35,6 +35,9 @@
 import sys
 import time
 import uuid
+import json
+import tempfile
+import pathlib
 
 
 class Address:
@@ -230,6 +233,7 @@
 def __init__(self, path, uuid=None):
 self.path = path
 self.resolved_path = None
+self.resolve = False
 self.resolved = False
 self.unavailable = False
 self.uuid = uuid
@@ -240,6 +244,7 @@
 self.module = None
 self.symfile = None
 self.slide = None
+self.symbols = dict()
 
 @classmethod
 def InitWithSBTargetAndSBModule(cls, target, module):
@@ -372,14 +377,34 @@
 uuid_str = self.get_normalized_uuid_string()
 if uuid_str:
 self.module = target.AddModule(None, None, uuid_str)
-if not self.module:
+if not self.module and self.resolve:
 self.locate_module_and_debug_symbols()
-if self.unavailable:
-return None
-resolved_path = self.get_resolved_path()
-self.module = target.AddModule(
-resolved_path, None, uuid_str, self.symfile)
-if not self.module:
+if not self.unavailable:
+resolved_path = self.get_resolved_path()
+self.module = target.AddModule(
+resolved_path, None, uuid_str, self.symfile)
+if not self.module and self.section_infos:
+name = pathlib.Path(self.path).name
+with tempfile.NamedTemporaryFile(suffix='.' + name) as tf:
+data = {
+'triple': target.triple,
+'uuid': uuid_str,
+'type': 'sharedlibrary',
+'sections': list(),
+'symbols': list()
+}
+for section in self.section_infos:
+data['sections'].append({
+'name' : section.name,
+'address': 0,
+'size': section.end_addr - section.start_addr
+})
+for symbol in self.symbols.values():
+data['symbols'].append(symbol)
+with open(tf.name, 'w') as f:
+f.write(json.dumps(data, indent=4))
+self.module = target.AddModule(tf.name, None, uuid_str)
+if not self.module and not self.unavailable:
 return 'error: unable to get module for (%s) "%s"' % (
 self.arch, self.get_resolved_path())
 if self.has_section_load_info():
Index:

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

2023-04-12 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 513025.
JDevlieghere retitled this revision from "[lldb] Use ObjectFileJSON to create 
modules for interactive crashlogs (WIP)" to "[lldb] Use ObjectFileJSON to 
create modules for interactive crashlogs".
JDevlieghere edited the summary of this revision.

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

https://reviews.llvm.org/D148172

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/symbolication.py
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test

Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
@@ -41,3 +41,5 @@
 # CHECK-NEXT:frame #2: 0x000100ec5a87 multithread-test`compute_pow{{.*}} [artificial]
 # CHECK:frame #{{[0-9]+}}: 0x00019cc7e06b{{.*}} [artificial]
 # CHECK:frame #{{[0-9]+}}: 0x00019cc78e2b{{.*}} [artificial]
+
+image list
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
@@ -478,6 +478,15 @@
   "source" : "A",
   "base" : 0,
   "uuid" : "----"
+},
+{
+  "arch": "arm64",
+  "base": 12345,
+  "name": "bogus.dylib",
+  "path": "/usr/lib/system/bogus.dylib",
+  "size": 1000,
+  "source": "P",
+  "uuid": "----"
 }
   ],
   "userID": 501,
Index: lldb/examples/python/symbolication.py
===
--- lldb/examples/python/symbolication.py
+++ lldb/examples/python/symbolication.py
@@ -35,6 +35,9 @@
 import sys
 import time
 import uuid
+import json
+import tempfile
+import pathlib
 
 
 class Address:
@@ -230,6 +233,7 @@
 def __init__(self, path, uuid=None):
 self.path = path
 self.resolved_path = None
+self.resolve = False
 self.resolved = False
 self.unavailable = False
 self.uuid = uuid
@@ -240,6 +244,7 @@
 self.module = None
 self.symfile = None
 self.slide = None
+self.symbols = dict()
 
 @classmethod
 def InitWithSBTargetAndSBModule(cls, target, module):
@@ -372,14 +377,34 @@
 uuid_str = self.get_normalized_uuid_string()
 if uuid_str:
 self.module = target.AddModule(None, None, uuid_str)
-if not self.module:
+if not self.module and self.resolve:
 self.locate_module_and_debug_symbols()
-if self.unavailable:
-return None
-resolved_path = self.get_resolved_path()
-self.module = target.AddModule(
-resolved_path, None, uuid_str, self.symfile)
-if not self.module:
+if not self.unavailable:
+resolved_path = self.get_resolved_path()
+self.module = target.AddModule(
+resolved_path, None, uuid_str, self.symfile)
+if not self.module and self.section_infos:
+name = pathlib.Path(self.path).name
+with tempfile.NamedTemporaryFile(suffix='.' + name) as tf:
+data = {
+'triple': target.triple,
+'uuid': uuid_str,
+'type': 'sharedlibrary',
+'sections': list(),
+'symbols': list()
+}
+for section in self.section_infos:
+data['sections'].append({
+'name' : section.name,
+'address': 0,
+'size': section.end_addr - section.start_addr
+})
+for symbol in self.symbols.values():
+data['symbols'].append(symbol)
+with open(tf.name, 'w') as f:
+f.write(json.dumps(data, indent=4))
+self.module = target.AddModule(tf.name, None, uuid_str)
+if not self.module and not self.unavailable:
 return 'error: unable to get module for (%s) "%s"' % (
 self.arch, self.get_resolved_path())

[Lldb-commits] [PATCH] D147816: Clarify how watchpoint description in stop packets work, fix AArch64 unintended behavior

2023-04-12 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda added a comment.

FWIW to show the motivation for this patch (which, again comes down to removing 
`core >= ArchSpec::eCore_arm_generic && core <= ArchSpec::eCore_arm_aarch64` 
when creating the StopInfo from the watchpoint description), the test case I 
wrote in https://reviews.llvm.org/D147820 shows the problem where I adopt the 
lldb-server mechanism of watchpoint reporting on Darwin systems and I hit this 
problem:

  #include 
  #include 
  int main() {
union {
  uint8_t buf[8];
  uint64_t val;
} a;
a.val = 0;  // break here
for (int i = 0; i < 5; i++) {
  a.val = i;
  printf ("a.val is %lu\n", a.val);
}
  }
  
  (lldb) br s -p break
  (lldb) r
  Process 2182 stopped
  -> 8a.val = 0;  // break here
  
  (lldb) w s v a.buf[2]
  Watchpoint created: Watchpoint 1: addr = 0xf3a2 size = 1 state = 
enabled type = w
  declare @ '/home/jason/a.c:7'
  watchpoint spec = 'a.buf[2]'
  new value: '\xff'
  (lldb) c
  Process 2182 resuming
  a.val is 0
  a.val is 1
  a.val is 2
  a.val is 3
  a.val is 4
  Process 2182 exited with status = 0 (0x) 
  (lldb) 

These watchpoint hits are silently skipped over on AArch64.  If you turn on the 
packet log, you'll see the watchpoint hits reported by debugserver, but lldb 
silently skips them.  This is correct behavior on MIPS; ARM targets using 
lldb-server should not have opted into this behavior.  When we set aside the 
comments and variable renaming, that's what we're fixing with this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147816

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

[Lldb-commits] [lldb] 9cbdfcd - [lldb] Fix assertion when ScriptedProcess have no pid after launch

2023-04-12 Thread Med Ismail Bennani via lldb-commits 

Author: Med Ismail Bennani
Date: 2023-04-12T16:04:22-07:00
New Revision: 9cbdfcdb4cf75bdb4f7061276ff89929c2b157a1

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

LOG: [lldb] Fix assertion when ScriptedProcess have no pid after launch

This patch should fix an assertion that causes some test failures:
https://ci.swift.org/view/LLDB/job/llvm-org-lldb-release-debuginfo/3587/console

This was caused by the changes introduces in `88f409194d5a` where we
replaced `DidLaunch` by `DidResume` in the `ScriptedProcess` class.

However, by the time we resume the process, the pid should be already
set. To address this, this patch brings back `DidLaunch` which will
initialize the ScriptedProcess pid with a placeholder value. That value
will be updated in `DidResume` to the final pid.

Note, this 2 stage PID initialization is necessary sometimes, when the
scripted process gets stopped at entry (launch) and gets assigned an
object that contains the PID value. In this case, we need to update the
PID when we resume the process after we've stopped at entry.

This also replaces the default scripted process id to an arbitrary
number (42) since the current value (0) is considered invalid.

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/examples/python/scripted_process/crashlog_scripted_process.py
lldb/examples/python/scripted_process/scripted_process.py
lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
lldb/source/Plugins/Process/scripted/ScriptedProcess.h

Removed: 




diff  --git 
a/lldb/examples/python/scripted_process/crashlog_scripted_process.py 
b/lldb/examples/python/scripted_process/crashlog_scripted_process.py
index b8c05717bb3a9..236853e826c56 100644
--- a/lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ b/lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -100,9 +100,6 @@ def get_loaded_images(self):
 # from it.
 return self.loaded_images
 
-def get_process_id(self) -> int:
-return self.pid
-
 def should_stop(self) -> bool:
 return True
 

diff  --git a/lldb/examples/python/scripted_process/scripted_process.py 
b/lldb/examples/python/scripted_process/scripted_process.py
index 044aee1338808..e4d25214da766 100644
--- a/lldb/examples/python/scripted_process/scripted_process.py
+++ b/lldb/examples/python/scripted_process/scripted_process.py
@@ -47,6 +47,7 @@ def __init__(self, exe_ctx, args):
 self.loaded_images = []
 self.metadata = {}
 self.capabilities = {}
+self.pid = 42
 
 def get_capabilities(self):
 """ Get a dictionary containing the process capabilities.
@@ -138,7 +139,7 @@ def get_process_id(self):
 Returns:
 int: The scripted process identifier.
 """
-return 0
+return self.pid
 
 def launch(self):
 """ Simulate the scripted process launch.

diff  --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp 
b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
index 999edd8f8db71..23ff8811e3fd7 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -168,7 +168,10 @@ Status ScriptedProcess::DoLaunch(Module *exe_module,
   return {};
 }
 
+void ScriptedProcess::DidLaunch() { m_pid = GetInterface().GetProcessID(); }
+
 void ScriptedProcess::DidResume() {
+  // Update the PID again, in case the user provided a placeholder pid at 
launch
   m_pid = GetInterface().GetProcessID();
   GetLoadedDynamicLibrariesInfos();
 }

diff  --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h 
b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
index 856e05c901ed5..60f42cbaf7f2c 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -47,6 +47,8 @@ class ScriptedProcess : public Process {
 
   Status DoLaunch(Module *exe_module, ProcessLaunchInfo _info) override;
 
+  void DidLaunch() override;
+
   void DidResume() override;
 
   Status DoResume() override;



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

[Lldb-commits] [PATCH] D148153: [lldb] Fix assertion when ScriptedProcess have no pid after launch

2023-04-12 Thread Med Ismail Bennani via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9cbdfcdb4cf7: [lldb] Fix assertion when ScriptedProcess have 
no pid after launch (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148153

Files:
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -47,6 +47,8 @@
 
   Status DoLaunch(Module *exe_module, ProcessLaunchInfo _info) override;
 
+  void DidLaunch() override;
+
   void DidResume() override;
 
   Status DoResume() override;
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -168,7 +168,10 @@
   return {};
 }
 
+void ScriptedProcess::DidLaunch() { m_pid = GetInterface().GetProcessID(); }
+
 void ScriptedProcess::DidResume() {
+  // Update the PID again, in case the user provided a placeholder pid at 
launch
   m_pid = GetInterface().GetProcessID();
   GetLoadedDynamicLibrariesInfos();
 }
Index: lldb/examples/python/scripted_process/scripted_process.py
===
--- lldb/examples/python/scripted_process/scripted_process.py
+++ lldb/examples/python/scripted_process/scripted_process.py
@@ -47,6 +47,7 @@
 self.loaded_images = []
 self.metadata = {}
 self.capabilities = {}
+self.pid = 42
 
 def get_capabilities(self):
 """ Get a dictionary containing the process capabilities.
@@ -138,7 +139,7 @@
 Returns:
 int: The scripted process identifier.
 """
-return 0
+return self.pid
 
 def launch(self):
 """ Simulate the scripted process launch.
Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -100,9 +100,6 @@
 # from it.
 return self.loaded_images
 
-def get_process_id(self) -> int:
-return self.pid
-
 def should_stop(self) -> bool:
 return True
 


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -47,6 +47,8 @@
 
   Status DoLaunch(Module *exe_module, ProcessLaunchInfo _info) override;
 
+  void DidLaunch() override;
+
   void DidResume() override;
 
   Status DoResume() override;
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -168,7 +168,10 @@
   return {};
 }
 
+void ScriptedProcess::DidLaunch() { m_pid = GetInterface().GetProcessID(); }
+
 void ScriptedProcess::DidResume() {
+  // Update the PID again, in case the user provided a placeholder pid at launch
   m_pid = GetInterface().GetProcessID();
   GetLoadedDynamicLibrariesInfos();
 }
Index: lldb/examples/python/scripted_process/scripted_process.py
===
--- lldb/examples/python/scripted_process/scripted_process.py
+++ lldb/examples/python/scripted_process/scripted_process.py
@@ -47,6 +47,7 @@
 self.loaded_images = []
 self.metadata = {}
 self.capabilities = {}
+self.pid = 42
 
 def get_capabilities(self):
 """ Get a dictionary containing the process capabilities.
@@ -138,7 +139,7 @@
 Returns:
 int: The scripted process identifier.
 """
-return 0
+return self.pid
 
 def launch(self):
 """ Simulate the scripted process launch.
Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -100,9 +100,6 @@
 # from it.
 return self.loaded_images
 
-def get_process_id(self) -> int:
-return self.pid
-
 def should_stop(self) -> bool:

[Lldb-commits] [PATCH] D148175: [lldb] Add `operator StringRef` to ConstString

2023-04-12 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added a comment.

I understand that the opposite direction is explicit because actual work is 
being done. In this direction, it shouldn't affect memory management (since 
ConstStrings live forever) or performance, so I think this is good (and very 
convenient!).
Does anyone else see a problem with this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148175

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

[Lldb-commits] [PATCH] D148175: [lldb] Add `operator StringRef` to ConstString

2023-04-12 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione created this revision.
kastiglione added reviewers: aprantl, JDevlieghere, jingham.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Add a `StringRef` conversion function to `ConstString`.

This will make using llvm and other non-ConstString APIs more convenient.

For demonstration, this updates Module.cpp to demonstrate.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148175

Files:
  lldb/include/lldb/Utility/ConstString.h
  lldb/source/Core/Module.cpp


Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -748,8 +748,7 @@
   // relatively inexpensive since no demangling is actually occuring. See
   // Mangled::SetValue for more context.
   const bool function_name_may_be_mangled =
-  Mangled::GetManglingScheme(function_name.GetStringRef()) !=
-  Mangled::eManglingSchemeNone;
+  Mangled::GetManglingScheme(function_name) != 
Mangled::eManglingSchemeNone;
   ConstString demangled_function_name = function_name;
   if (function_name_may_be_mangled) {
 Mangled mangled_function_name(function_name);
@@ -760,11 +759,10 @@
   // Otherwise just check that the demangled function name contains the
   // demangled user-provided name.
   if (Language *language = Language::FindPlugin(language_type))
-return language->DemangledNameContainsPath(m_name.GetStringRef(),
-   demangled_function_name);
+return language->DemangledNameContainsPath(m_name, 
demangled_function_name);
 
-  llvm::StringRef function_name_ref = demangled_function_name.GetStringRef();
-  return function_name_ref.contains(m_name.GetStringRef());
+  llvm::StringRef function_name_ref = demangled_function_name;
+  return function_name_ref.contains(m_name);
 }
 
 void Module::LookupInfo::Prune(SymbolContextList _list,
@@ -803,7 +801,7 @@
 CPlusPlusLanguage::MethodName cpp_method(full_name);
 if (cpp_method.IsValid()) {
   if (cpp_method.GetContext().empty()) {
-if (cpp_method.GetBasename().compare(m_name.GetStringRef()) != 0) {
+if (cpp_method.GetBasename().compare(m_name) != 0) {
   sc_list.RemoveContextAtIndex(i);
   continue;
 }
@@ -1026,8 +1024,8 @@
   FindTypes_Impl(name, CompilerDeclContext(), UINT_MAX,
  searched_symbol_files, typesmap);
   if (exact_match) {
-typesmap.RemoveMismatchedTypes(type_scope, name.GetStringRef(),
-   type_class, exact_match);
+typesmap.RemoveMismatchedTypes(type_scope, name, type_class,
+   exact_match);
   }
 }
   }
@@ -1132,7 +1130,7 @@
 return;
 
   StreamString ss;
-  ss << file_name.GetStringRef()
+  ss << file_name
  << " was compiled with optimization - stepping may behave "
 "oddly; variables may not be available.";
   Debugger::ReportWarning(std::string(ss.GetString()), debugger_id,
@@ -1668,7 +1666,7 @@
   llvm::raw_string_ostream id_strm(identifier);
   id_strm << m_arch.GetTriple().str() << '-' << m_file.GetPath();
   if (m_object_name)
-id_strm << '(' << m_object_name.GetStringRef() << ')';
+id_strm << '(' << m_object_name << ')';
   if (m_object_offset > 0)
 id_strm << m_object_offset;
   const auto mtime = llvm::sys::toTimeT(m_object_mod_time);
@@ -1682,7 +1680,7 @@
   llvm::raw_string_ostream strm(key);
   strm << m_arch.GetTriple().str() << '-' << m_file.GetFilename();
   if (m_object_name)
-strm << '(' << m_object_name.GetStringRef() << ')';
+strm << '(' << m_object_name << ')';
   strm << '-' << llvm::format_hex(Hash(), 10);
   return strm.str();
 }
Index: lldb/include/lldb/Utility/ConstString.h
===
--- lldb/include/lldb/Utility/ConstString.h
+++ lldb/include/lldb/Utility/ConstString.h
@@ -180,6 +180,9 @@
 
   bool operator<(ConstString rhs) const;
 
+  // Implicitly convert \class ConstString instances to \class StringRef.
+  operator llvm::StringRef() const { return GetStringRef(); }
+
   /// Get the string value as a C string.
   ///
   /// Get the value of the contained string as a NULL terminated C string


Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -748,8 +748,7 @@
   // relatively inexpensive since no demangling is actually occuring. See
   // Mangled::SetValue for more context.
   const bool function_name_may_be_mangled =
-  Mangled::GetManglingScheme(function_name.GetStringRef()) !=
-  Mangled::eManglingSchemeNone;
+  Mangled::GetManglingScheme(function_name) != Mangled::eManglingSchemeNone;
   ConstString demangled_function_name = function_name;

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs (WIP)

2023-04-12 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere created this revision.
JDevlieghere added a reviewer: mib.
Herald added a project: All.
JDevlieghere requested review of this revision.

se ObjectFileJSON to create modules for interactive crashlogs.

Currently still WIP as I need to update the textual and non-interactive 
variants.


https://reviews.llvm.org/D148172

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/symbolication.py

Index: lldb/examples/python/symbolication.py
===
--- lldb/examples/python/symbolication.py
+++ lldb/examples/python/symbolication.py
@@ -35,6 +35,9 @@
 import sys
 import time
 import uuid
+import json
+import tempfile
+import pathlib
 
 
 class Address:
@@ -230,6 +233,7 @@
 def __init__(self, path, uuid=None):
 self.path = path
 self.resolved_path = None
+self.resolve = False
 self.resolved = False
 self.unavailable = False
 self.uuid = uuid
@@ -240,6 +244,7 @@
 self.module = None
 self.symfile = None
 self.slide = None
+self.symbols = dict()
 
 @classmethod
 def InitWithSBTargetAndSBModule(cls, target, module):
@@ -372,13 +377,34 @@
 uuid_str = self.get_normalized_uuid_string()
 if uuid_str:
 self.module = target.AddModule(None, None, uuid_str)
-if not self.module:
+if not self.module and self.resolve:
 self.locate_module_and_debug_symbols()
 if self.unavailable:
 return None
 resolved_path = self.get_resolved_path()
 self.module = target.AddModule(
 resolved_path, None, uuid_str, self.symfile)
+if not self.module and self.section_infos:
+name = pathlib.Path(self.path).name
+with tempfile.NamedTemporaryFile(suffix='.' + name) as tf:
+data = {
+'triple': target.triple,
+'uuid': uuid_str,
+'type': 'sharedlibrary',
+'sections': list(),
+'symbols': list()
+}
+for section in self.section_infos:
+data['sections'].append({
+'name' : section.name,
+'address': 0,
+'size': section.end_addr - section.start_addr
+})
+for symbol in self.symbols.values():
+data['symbols'].append(symbol)
+with open(tf.name, 'w') as f:
+f.write(json.dumps(data, indent=4))
+self.module = target.AddModule(tf.name, None, uuid_str)
 if not self.module:
 return 'error: unable to get module for (%s) "%s"' % (
 self.arch, self.get_resolved_path())
Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -22,27 +22,28 @@
 if hasattr(self.crashlog, 'asb'):
 self.extended_thread_info = self.crashlog.asb
 
-def load_images(self, images):
-#TODO: Add to self.loaded_images and load images in lldb
-if images:
-for image in images:
-if image not in self.loaded_images:
-if image.uuid == uuid.UUID(int=0):
-continue
-err = image.add_module(self.target)
-if err:
-# Append to SBCommandReturnObject
-print(err)
-else:
-self.loaded_images.append(image)
+if self.load_all_images:
+for image in self.crashlog.images:
+image.resolve = True
+else:
+for thread in self.crashlog.threads:
+if thread.did_crash():
+for ident in thread.idents:
+for image in self.crashlog.find_images_with_identifier(ident):
+image.resolve = False
+
+for image in self.crashlog.images:
+if image not in self.loaded_images:
+if image.uuid == uuid.UUID(int=0):
+continue
+err = image.add_module(self.target)
+if err:
+# Append to SBCommandReturnObject
+print(err)
+else:
+self.loaded_images.append(image)
 
 for thread in self.crashlog.threads:
-if

[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

2023-04-12 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 512963.
JDevlieghere added a comment.

Fix bug in ObjectFileJSON where we wouldn't read the rest of the file and fail 
parsing.


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

https://reviews.llvm.org/D148062

Files:
  lldb/include/lldb/Core/Section.h
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/lldb/test/API/macosx/symbols/TestObjectFileJSON.py
  lldb/source/Core/Section.cpp
  lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
  lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.h
  lldb/source/Symbol/ObjectFile.cpp
  lldb/test/API/macosx/symbols/TestObjectFileJSON.py

Index: lldb/test/API/macosx/symbols/TestObjectFileJSON.py
===
--- /dev/null
+++ lldb/test/API/macosx/symbols/TestObjectFileJSON.py
@@ -0,0 +1,100 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+import json
+import uuid
+import os
+import shutil
+
+
+class TestObjectFileJSOn(TestBase):
+TRIPLE = "arm64-apple-macosx13.0.0"
+
+def setUp(self):
+TestBase.setUp(self)
+self.source = "main.c"
+
+def emitJSON(self, data, path):
+json_object = json.dumps(data, indent=4)
+json_object_file = self.getBuildArtifact("a.json")
+with open(json_object_file, "w") as outfile:
+outfile.write(json_object)
+
+def toModuleSpec(self, path):
+module_spec = lldb.SBModuleSpec()
+module_spec.SetFileSpec(lldb.SBFileSpec(path))
+return module_spec
+
+@no_debug_info_test
+def test_target(self):
+data = {
+"triple": self.TRIPLE,
+"uuid": str(uuid.uuid4()),
+"type": "executable",
+}
+
+json_object_file = self.getBuildArtifact("a.json")
+self.emitJSON(data, json_object_file)
+
+target = self.dbg.CreateTarget(json_object_file)
+self.assertTrue(target.IsValid())
+self.assertEqual(target.GetTriple(), self.TRIPLE)
+
+@no_debug_info_test
+def test_module(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+target = self.dbg.CreateTarget(exe)
+
+data = {
+"triple": self.TRIPLE,
+"uuid": str(uuid.uuid4()),
+}
+
+json_object_file = self.getBuildArtifact("a.json")
+self.emitJSON(data, json_object_file)
+
+module = target.AddModule(self.toModuleSpec(json_object_file))
+self.assertFalse(module.IsValid())
+
+data = {
+"triple": self.TRIPLE,
+"uuid": str(uuid.uuid4()),
+"type": "sharedlibrary",
+"sections": [
+{
+"name": "__TEXT",
+"type": "code",
+"address": 0,
+"size": 0x222,
+}
+],
+"symbols": [
+{
+"name": "foo",
+"address": 0x100,
+"size": 0x11,
+}
+],
+}
+self.emitJSON(data, json_object_file)
+
+module = target.AddModule(self.toModuleSpec(json_object_file))
+self.assertTrue(module.IsValid())
+
+section = module.GetSectionAtIndex(0)
+self.assertTrue(section.IsValid())
+self.assertEqual(section.GetName(), "__TEXT")
+self.assertEqual(section.file_addr, 0x0)
+self.assertEqual(section.size, 0x222)
+
+symbol = module.FindSymbol("foo")
+self.assertTrue(symbol.IsValid())
+self.assertEqual(symbol.addr.GetFileAddress(), 0x100)
+self.assertEqual(symbol.GetSize(), 0x11)
+
+error = target.SetSectionLoadAddress(section, 0x1000)
+self.assertSuccess(error)
+self.assertEqual(symbol.addr.GetLoadAddress(target), 0x1100)
Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -761,3 +761,34 @@
   m_cache_hash = llvm::djbHash(strm.GetString());
   return *m_cache_hash;
 }
+
+namespace llvm {
+namespace json {
+
+bool fromJSON(const llvm::json::Value ,
+  lldb_private::ObjectFile::Type , llvm::json::Path path) {
+  if (auto str = value.getAsString()) {
+type = llvm::StringSwitch(*str)
+   .Case("corefile", ObjectFile::eTypeCoreFile)
+   .Case("executable", ObjectFile::eTypeExecutable)
+   .Case("debuginfo", ObjectFile::eTypeDebugInfo)
+   .Case("dynamiclinker", ObjectFile::eTypeDynamicLinker)
+   .Case("objectfile", ObjectFile::eTypeObjectFile)
+   .Case("sharedlibrary", ObjectFile::eTypeSharedLibrary)
+   .Case("stublibrary", ObjectFile::eTypeStubLibrary)
+   .Case("jit", ObjectFile::eTypeJIT)
+   .Case("unknown",

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

2023-04-12 Thread Pavel Kosov via Phabricator via lldb-commits 
kpdev42 updated this revision to Diff 512945.
kpdev42 added a comment.

Thanks for pointing out the bug @Michael137 . It seems that clang assigns 
arbitrary offsets for non_unique_address so analyzing them brings me nowhere. 
In this patch I tried assigning no_unique_address to every empty structure and 
this fixed issue you found, making the code changes much smaller and simpler. 
The lldb test suite pass for me as well


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/lang/cpp/no_unique_address/Makefile
  lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
  lldb/test/API/lang/cpp/no_unique_address/main.cpp

Index: lldb/test/API/lang/cpp/no_unique_address/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/no_unique_address/main.cpp
@@ -0,0 +1,77 @@
+struct C
+{
+ long c,d;
+};
+
+struct Q
+{
+ long h;
+};
+
+struct D
+{
+};
+
+struct B
+{
+  [[no_unique_address]] D x;
+};
+
+struct E
+{
+  [[no_unique_address]] D x;
+};
+
+struct Foo1 : B,E,C
+{
+ long a = 42,b = 52;
+} _f1;
+
+struct Foo2 : B,E
+{
+ long v = 42;
+} _f2;
+
+struct Foo3 : C,B,E
+{
+ long v = 42;
+} _f3;
+
+struct Foo4 : B,C,E,Q
+{
+ long v = 42;
+} _f4;
+
+struct Foo5 : B,C,E
+{
+ [[no_unique_address]] D x1;
+ [[no_unique_address]] D x2;
+ long v1 = 42;
+ [[no_unique_address]] D y1;
+ [[no_unique_address]] D y2;
+ long v2 = 52;
+ [[no_unique_address]] D z1;
+ [[no_unique_address]] D z2;
+} _f5;
+
+struct Foo6 : B,E
+{
+ long v1 = 42;
+ [[no_unique_address]] D y1;
+ [[no_unique_address]] D y2;
+ long v2 = 52;
+} _f6;
+
+namespace NS {
+template struct Wrap {};
+}
+
+struct Foo7 : NS::Wrap,B,E {
+  NS::Wrap w1;
+  B b1;
+  long v = 42;
+} _f7;
+
+int main() {
+  return 0; // Set breakpoint here.
+}
Index: lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
@@ -0,0 +1,33 @@
+"""
+Test that we correctly handle [[no_unique_address]] attribute.
+"""
+
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestInlineNamespace(TestBase):
+
+def test(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self,
+"// Set breakpoint here.", lldb.SBFileSpec("main.cpp"))
+
+
+self.expect_expr("_f3", result_type="Foo3")
+self.expect_expr("_f7", result_type="Foo7")
+
+self.expect_expr("_f1.a", result_type="long", result_value="42")
+self.expect_expr("_f1.b", result_type="long", result_value="52")
+self.expect_expr("_f2.v", result_type="long", result_value="42")
+self.expect_expr("_f3.v", result_type="long", result_value="42")
+self.expect_expr("_f4.v", result_type="long", result_value="42")
+self.expect_expr("_f5.v1", result_type="long", result_value="42")
+self.expect_expr("_f5.v2", result_type="long", result_value="52")
+self.expect_expr("_f6.v1", result_type="long", result_value="42")
+self.expect_expr("_f6.v2", result_type="long", result_value="52")
+self.expect_expr("_f7.v", result_type="long", result_value="42")
Index: lldb/test/API/lang/cpp/no_unique_address/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/no_unique_address/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1566,6 +1566,16 @@
   return qualified_name;
 }
 
+static bool IsEmptyStruct(const DWARFDIE ) {
+  if (!die.HasChildren())
+return true;
+
+  // Empty templates are actually empty structures.
+  return llvm::all_of(die.children(), [](const DWARFDIE ) {
+return die.Tag() == DW_TAG_template_type_parameter;
+  });
+}
+
 TypeSP
 DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext ,
const DWARFDIE ,
@@ -1829,7 +1839,7 @@
 // has child classes or types that require the class to be created
 // for use as their decl contexts the class will be ready to accept
 // these child definitions.
-if (!die.HasChildren()) {
+if (IsEmptyStruct(die)) {
   // No children for this struct/union/class, lets finish it
   if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
 TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
@@ -1852,6 +1862,9 @@
 clang::RecordDecl *record_decl =

[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

2023-04-12 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 512946.
JDevlieghere added a comment.

- Improve error reporting
- Fix bug


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

https://reviews.llvm.org/D148062

Files:
  lldb/include/lldb/Core/Section.h
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/lldb/test/API/macosx/symbols/TestObjectFileJSON.py
  lldb/source/Core/Section.cpp
  lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
  lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.h
  lldb/source/Symbol/ObjectFile.cpp
  lldb/test/API/macosx/symbols/TestObjectFileJSON.py

Index: lldb/test/API/macosx/symbols/TestObjectFileJSON.py
===
--- /dev/null
+++ lldb/test/API/macosx/symbols/TestObjectFileJSON.py
@@ -0,0 +1,100 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+import json
+import uuid
+import os
+import shutil
+
+
+class TestObjectFileJSOn(TestBase):
+TRIPLE = "arm64-apple-macosx13.0.0"
+
+def setUp(self):
+TestBase.setUp(self)
+self.source = "main.c"
+
+def emitJSON(self, data, path):
+json_object = json.dumps(data, indent=4)
+json_object_file = self.getBuildArtifact("a.json")
+with open(json_object_file, "w") as outfile:
+outfile.write(json_object)
+
+def toModuleSpec(self, path):
+module_spec = lldb.SBModuleSpec()
+module_spec.SetFileSpec(lldb.SBFileSpec(path))
+return module_spec
+
+@no_debug_info_test
+def test_target(self):
+data = {
+"triple": self.TRIPLE,
+"uuid": str(uuid.uuid4()),
+"type": "executable",
+}
+
+json_object_file = self.getBuildArtifact("a.json")
+self.emitJSON(data, json_object_file)
+
+target = self.dbg.CreateTarget(json_object_file)
+self.assertTrue(target.IsValid())
+self.assertEqual(target.GetTriple(), self.TRIPLE)
+
+@no_debug_info_test
+def test_module(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+target = self.dbg.CreateTarget(exe)
+
+data = {
+"triple": self.TRIPLE,
+"uuid": str(uuid.uuid4()),
+}
+
+json_object_file = self.getBuildArtifact("a.json")
+self.emitJSON(data, json_object_file)
+
+module = target.AddModule(self.toModuleSpec(json_object_file))
+self.assertFalse(module.IsValid())
+
+data = {
+"triple": self.TRIPLE,
+"uuid": str(uuid.uuid4()),
+"type": "sharedlibrary",
+"sections": [
+{
+"name": "__TEXT",
+"type": "code",
+"address": 0,
+"size": 0x222,
+}
+],
+"symbols": [
+{
+"name": "foo",
+"address": 0x100,
+"size": 0x11,
+}
+],
+}
+self.emitJSON(data, json_object_file)
+
+module = target.AddModule(self.toModuleSpec(json_object_file))
+self.assertTrue(module.IsValid())
+
+section = module.GetSectionAtIndex(0)
+self.assertTrue(section.IsValid())
+self.assertEqual(section.GetName(), "__TEXT")
+self.assertEqual(section.file_addr, 0x0)
+self.assertEqual(section.size, 0x222)
+
+symbol = module.FindSymbol("foo")
+self.assertTrue(symbol.IsValid())
+self.assertEqual(symbol.addr.GetFileAddress(), 0x100)
+self.assertEqual(symbol.GetSize(), 0x11)
+
+error = target.SetSectionLoadAddress(section, 0x1000)
+self.assertSuccess(error)
+self.assertEqual(symbol.addr.GetLoadAddress(target), 0x1100)
Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -761,3 +761,34 @@
   m_cache_hash = llvm::djbHash(strm.GetString());
   return *m_cache_hash;
 }
+
+namespace llvm {
+namespace json {
+
+bool fromJSON(const llvm::json::Value ,
+  lldb_private::ObjectFile::Type , llvm::json::Path path) {
+  if (auto str = value.getAsString()) {
+type = llvm::StringSwitch(*str)
+   .Case("corefile", ObjectFile::eTypeCoreFile)
+   .Case("executable", ObjectFile::eTypeExecutable)
+   .Case("debuginfo", ObjectFile::eTypeDebugInfo)
+   .Case("dynamiclinker", ObjectFile::eTypeDynamicLinker)
+   .Case("objectfile", ObjectFile::eTypeObjectFile)
+   .Case("sharedlibrary", ObjectFile::eTypeSharedLibrary)
+   .Case("stublibrary", ObjectFile::eTypeStubLibrary)
+   .Case("jit", ObjectFile::eTypeJIT)
+   .Case("unknown", ObjectFile::eTypeUnknown)
+

[Lldb-commits] [PATCH] D148153: [lldb] Fix assertion when ScriptedProcess have no pid after launch

2023-04-12 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib updated this revision to Diff 512927.

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

https://reviews.llvm.org/D148153

Files:
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -47,6 +47,8 @@
 
   Status DoLaunch(Module *exe_module, ProcessLaunchInfo _info) override;
 
+  void DidLaunch() override;
+
   void DidResume() override;
 
   Status DoResume() override;
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -168,7 +168,10 @@
   return {};
 }
 
+void ScriptedProcess::DidLaunch() { m_pid = GetInterface().GetProcessID(); }
+
 void ScriptedProcess::DidResume() {
+  // Update the PID again, in case the user provided a placeholder pid at 
launch
   m_pid = GetInterface().GetProcessID();
   GetLoadedDynamicLibrariesInfos();
 }
Index: lldb/examples/python/scripted_process/scripted_process.py
===
--- lldb/examples/python/scripted_process/scripted_process.py
+++ lldb/examples/python/scripted_process/scripted_process.py
@@ -47,6 +47,7 @@
 self.loaded_images = []
 self.metadata = {}
 self.capabilities = {}
+self.pid = 42
 
 def get_capabilities(self):
 """ Get a dictionary containing the process capabilities.
@@ -138,7 +139,7 @@
 Returns:
 int: The scripted process identifier.
 """
-return 0
+return self.pid
 
 def launch(self):
 """ Simulate the scripted process launch.
Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -100,9 +100,6 @@
 # from it.
 return self.loaded_images
 
-def get_process_id(self) -> int:
-return self.pid
-
 def should_stop(self) -> bool:
 return True
 


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -47,6 +47,8 @@
 
   Status DoLaunch(Module *exe_module, ProcessLaunchInfo _info) override;
 
+  void DidLaunch() override;
+
   void DidResume() override;
 
   Status DoResume() override;
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -168,7 +168,10 @@
   return {};
 }
 
+void ScriptedProcess::DidLaunch() { m_pid = GetInterface().GetProcessID(); }
+
 void ScriptedProcess::DidResume() {
+  // Update the PID again, in case the user provided a placeholder pid at launch
   m_pid = GetInterface().GetProcessID();
   GetLoadedDynamicLibrariesInfos();
 }
Index: lldb/examples/python/scripted_process/scripted_process.py
===
--- lldb/examples/python/scripted_process/scripted_process.py
+++ lldb/examples/python/scripted_process/scripted_process.py
@@ -47,6 +47,7 @@
 self.loaded_images = []
 self.metadata = {}
 self.capabilities = {}
+self.pid = 42
 
 def get_capabilities(self):
 """ Get a dictionary containing the process capabilities.
@@ -138,7 +139,7 @@
 Returns:
 int: The scripted process identifier.
 """
-return 0
+return self.pid
 
 def launch(self):
 """ Simulate the scripted process launch.
Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -100,9 +100,6 @@
 # from it.
 return self.loaded_images
 
-def get_process_id(self) -> int:
-return self.pid
-
 def should_stop(self) -> bool:
 return True
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D148153: [lldb] Fix assertion when ScriptedProcess have no pid after launch

2023-04-12 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib created this revision.
mib added a reviewer: JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch should fix an assertion that causes some test failures:
https://ci.swift.org/view/LLDB/job/llvm-org-lldb-release-debuginfo/3587/console

This was caused by the changes introduces in `88f409194d5a` where we
replaced `DidLaunch` by `DidResume` in the `ScriptedProcess` class.

However, by the time we resume the process, the pid should be already
set. To address this, this patch brings back `DidLaunch` which will
initialize the ScriptedProcess pid with a placeholder value. That value
will be updated in `DidResume` to the final pid.

Note, this 2 stage PID initialization is necessary sometimes, when the
scripted process gets stopped at entry (launch) and gets assigned an
object that contains the PID value. In this case, we need to update the
PID when we resume the process after we've stopped at entry.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148153

Files:
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -47,6 +47,8 @@
 
   Status DoLaunch(Module *exe_module, ProcessLaunchInfo _info) override;
 
+  void DidLaunch() override;
+
   void DidResume() override;
 
   Status DoResume() override;
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -168,7 +168,10 @@
   return {};
 }
 
+void ScriptedProcess::DidLaunch() { m_pid = GetInterface().GetProcessID(); }
+
 void ScriptedProcess::DidResume() {
+  // Update the PID again, in case the user provided a placeholder pid at 
launch
   m_pid = GetInterface().GetProcessID();
   GetLoadedDynamicLibrariesInfos();
 }


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -47,6 +47,8 @@
 
   Status DoLaunch(Module *exe_module, ProcessLaunchInfo _info) override;
 
+  void DidLaunch() override;
+
   void DidResume() override;
 
   Status DoResume() override;
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -168,7 +168,10 @@
   return {};
 }
 
+void ScriptedProcess::DidLaunch() { m_pid = GetInterface().GetProcessID(); }
+
 void ScriptedProcess::DidResume() {
+  // Update the PID again, in case the user provided a placeholder pid at launch
   m_pid = GetInterface().GetProcessID();
   GetLoadedDynamicLibrariesInfos();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D148050: [lldb] Change formatter helper function parameter list to remove ConstString

2023-04-12 Thread Alex Langford via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0e22b8cba786: [lldb] Change formatter helper function 
parameter list to remove ConstString (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148050

Files:
  lldb/include/lldb/DataFormatters/FormattersHelpers.h
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/source/DataFormatters/FormattersHelpers.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp

Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -297,23 +297,23 @@
   objc_flags.SetSkipPointers(true);
   AddCXXSummary(objc_category_sp,
 lldb_private::formatters::ObjCSELSummaryProvider,
-"SEL summary provider", ConstString("SEL"), objc_flags);
-  AddCXXSummary(
-  objc_category_sp, lldb_private::formatters::ObjCSELSummaryProvider,
-  "SEL summary provider", ConstString("struct objc_selector"), objc_flags);
-  AddCXXSummary(
-  objc_category_sp, lldb_private::formatters::ObjCSELSummaryProvider,
-  "SEL summary provider", ConstString("objc_selector"), objc_flags);
-  AddCXXSummary(
-  objc_category_sp, lldb_private::formatters::ObjCSELSummaryProvider,
-  "SEL summary provider", ConstString("objc_selector *"), objc_flags);
+"SEL summary provider", "SEL", objc_flags);
+  AddCXXSummary(objc_category_sp,
+lldb_private::formatters::ObjCSELSummaryProvider,
+"SEL summary provider", "struct objc_selector", objc_flags);
+  AddCXXSummary(objc_category_sp,
+lldb_private::formatters::ObjCSELSummaryProvider,
+"SEL summary provider", "objc_selector", objc_flags);
   AddCXXSummary(objc_category_sp,
 lldb_private::formatters::ObjCSELSummaryProvider,
-"SEL summary provider", ConstString("SEL *"), objc_flags);
+"SEL summary provider", "objc_selector *", objc_flags);
+  AddCXXSummary(objc_category_sp,
+lldb_private::formatters::ObjCSELSummaryProvider,
+"SEL summary provider", "SEL *", objc_flags);
 
   AddCXXSummary(objc_category_sp,
 lldb_private::formatters::ObjCClassSummaryProvider,
-"Class summary provider", ConstString("Class"), objc_flags);
+"Class summary provider", "Class", objc_flags);
 
   SyntheticChildren::Flags class_synth_flags;
   class_synth_flags.SetCascades(true).SetSkipPointers(false).SetSkipReferences(
@@ -321,61 +321,62 @@
 
   AddCXXSynthetic(objc_category_sp,
   lldb_private::formatters::ObjCClassSyntheticFrontEndCreator,
-  "Class synthetic children", ConstString("Class"),
-  class_synth_flags);
+  "Class synthetic children", "Class", class_synth_flags);
 
   objc_flags.SetSkipPointers(false);
   objc_flags.SetCascades(true);
   objc_flags.SetSkipReferences(false);
 
   AddStringSummary(objc_category_sp, "${var.__FuncPtr%A}",
-   ConstString("__block_literal_generic"), objc_flags);
+   "__block_literal_generic", objc_flags);
 
-  AddStringSummary(objc_category_sp, "${var.years} years, ${var.months} "
- "months, ${var.days} days, ${var.hours} "
- "hours, ${var.minutes} minutes "
- "${var.seconds} seconds",
-   ConstString("CFGregorianUnits"), objc_flags);
   AddStringSummary(objc_category_sp,
-   "location=${var.location} length=${var.length}",
-   ConstString("CFRange"), objc_flags);
+   "${var.years} years, ${var.months} "
+   "months, ${var.days} days, ${var.hours} "
+   "hours, ${var.minutes} minutes "
+   "${var.seconds} seconds",
+   "CFGregorianUnits", objc_flags);
+  AddStringSummary(objc_category_sp,
+   "location=${var.location} length=${var.length}", "CFRange",
+   objc_flags);
 
   AddStringSummary(objc_category_sp,
-   "location=${var.location}, length=${var.length}",
-   ConstString("NSRange"), objc_flags);
+   "location=${var.location}, length=${var.length}", "NSRange",
+   objc_flags);
   AddStringSummary(objc_category_sp, "(${var.origin}, ${var.size}), ...",
-   ConstString("NSRectArray"), objc_flags);
+   "NSRectArray", objc_flags);
 
-  AddOneLineSummary(objc_category_sp, ConstString("NSPoint"), objc_flags);
-  AddOneLineSummary(objc_category_sp,

[Lldb-commits] [lldb] 0e22b8c - [lldb] Change formatter helper function parameter list to remove ConstString

2023-04-12 Thread Alex Langford via lldb-commits 

Author: Alex Langford
Date: 2023-04-12T11:20:24-07:00
New Revision: 0e22b8cba786a5da61b9279c4d2d10c756392a2e

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

LOG: [lldb] Change formatter helper function parameter list to remove 
ConstString

All of these functions take a ConstString for the type_name,
but this isn't really needed for two reasons:
1.) This parameter is always constructed from a static c-string
  constant.
2.) They are passed along to to `AddTypeSummary` as a StringRef anyway.

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

Added: 


Modified: 
lldb/include/lldb/DataFormatters/FormattersHelpers.h
lldb/source/DataFormatters/FormatManager.cpp
lldb/source/DataFormatters/FormattersHelpers.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp

Removed: 




diff  --git a/lldb/include/lldb/DataFormatters/FormattersHelpers.h 
b/lldb/include/lldb/DataFormatters/FormattersHelpers.h
index 4f8f0e8455cd9..e9af6656e3d7d 100644
--- a/lldb/include/lldb/DataFormatters/FormattersHelpers.h
+++ b/lldb/include/lldb/DataFormatters/FormattersHelpers.h
@@ -21,38 +21,38 @@
 namespace lldb_private {
 namespace formatters {
 void AddFormat(TypeCategoryImpl::SharedPointer category_sp, lldb::Format 
format,
-   ConstString type_name, TypeFormatImpl::Flags flags,
+   llvm::StringRef type_name, TypeFormatImpl::Flags flags,
bool regex = false);
 
 void AddSummary(TypeCategoryImpl::SharedPointer category_sp,
-lldb::TypeSummaryImplSP summary_sp, ConstString type_name,
+lldb::TypeSummaryImplSP summary_sp, llvm::StringRef type_name,
 bool regex = false);
 
 void AddStringSummary(TypeCategoryImpl::SharedPointer category_sp,
-  const char *string, ConstString type_name,
+  const char *string, llvm::StringRef type_name,
   TypeSummaryImpl::Flags flags, bool regex = false);
 
 void AddOneLineSummary(TypeCategoryImpl::SharedPointer category_sp,
-   ConstString type_name, TypeSummaryImpl::Flags flags,
+   llvm::StringRef type_name, TypeSummaryImpl::Flags flags,
bool regex = false);
 
 /// Add a summary that is implemented by a C++ callback.
 void AddCXXSummary(TypeCategoryImpl::SharedPointer category_sp,
CXXFunctionSummaryFormat::Callback funct,
-   const char *description, ConstString type_name,
+   const char *description, llvm::StringRef type_name,
TypeSummaryImpl::Flags flags, bool regex = false);
 
 /// Add a synthetic that is implemented by a C++ callback.
 void AddCXXSynthetic(TypeCategoryImpl::SharedPointer category_sp,
  CXXSyntheticChildren::CreateFrontEndCallback generator,
- const char *description, ConstString type_name,
+ const char *description, llvm::StringRef type_name,
  ScriptedSyntheticChildren::Flags flags,
  bool regex = false);
 
 void AddFilter(TypeCategoryImpl::SharedPointer category_sp,
std::vector children, const char *description,
-   ConstString type_name, ScriptedSyntheticChildren::Flags flags,
-   bool regex = false);
+   llvm::StringRef type_name,
+   ScriptedSyntheticChildren::Flags flags, bool regex = false);
 
 size_t ExtractIndexFromString(const char *item_name);
 

diff  --git a/lldb/source/DataFormatters/FormatManager.cpp 
b/lldb/source/DataFormatters/FormatManager.cpp
index 166264df99337..8387ce1121fd9 100644
--- a/lldb/source/DataFormatters/FormatManager.cpp
+++ b/lldb/source/DataFormatters/FormatManager.cpp
@@ -740,7 +740,7 @@ void FormatManager::LoadSystemFormatters() {
   fourchar_flags.SetCascades(true).SetSkipPointers(true).SetSkipReferences(
   true);
 
-  AddFormat(sys_category_sp, lldb::eFormatOSType, ConstString("FourCharCode"),
+  AddFormat(sys_category_sp, lldb::eFormatOSType, "FourCharCode",
 fourchar_flags);
 }
 
@@ -757,32 +757,19 @@ void FormatManager::LoadVectorFormatters() {
   .SetShowMembersOneLiner(true)
   .SetHideItemNames(true);
 
-  AddStringSummary(vectors_category_sp, "${var.uint128}",
-   ConstString("builtin_type_vec128"), vector_flags);
-  AddStringSummary(vectors_category_sp, "", ConstString("float[4]"),
-   vector_flags);
-  AddStringSummary(vectors_category_sp, "", ConstString("int32_t[4]"),
-   vector_flags);
-  AddStringSummary(vectors_category_sp, "", ConstString("int16_t[8]"),
-   vector_flags);
-

[Lldb-commits] [PATCH] D146965: [lldb] Add support for MSP430 in LLDB.

2023-04-12 Thread Ilia Kuklin via Phabricator via lldb-commits 
kuilpd updated this revision to Diff 512891.
kuilpd marked 6 inline comments as done.
kuilpd added a comment.

Added comments, changed memory strings formatting, added disassembler check.


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

https://reviews.llvm.org/D146965

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/include/lldb/Utility/DataExtractor.h
  lldb/source/Expression/IRMemoryMap.cpp
  lldb/source/Expression/LLVMUserExpression.cpp
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/ABI/CMakeLists.txt
  lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.cpp
  lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.h
  lldb/source/Plugins/ABI/MSP430/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterFallback.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestMSP430MSPDebug.py
  lldb/test/API/functionalities/gdb_remote_client/msp430.yaml
  lldb/unittests/Utility/ArchSpecTest.cpp

Index: lldb/unittests/Utility/ArchSpecTest.cpp
===
--- lldb/unittests/Utility/ArchSpecTest.cpp
+++ lldb/unittests/Utility/ArchSpecTest.cpp
@@ -123,6 +123,12 @@
   EXPECT_STREQ("i686", AS.GetArchitectureName());
   EXPECT_EQ(ArchSpec::eCore_x86_32_i686, AS.GetCore());
 
+  AS = ArchSpec();
+  EXPECT_TRUE(AS.SetTriple("msp430---elf"));
+  EXPECT_EQ(llvm::Triple::msp430, AS.GetTriple().getArch());
+  EXPECT_STREQ("msp430", AS.GetArchitectureName());
+  EXPECT_EQ(ArchSpec::eCore_msp430, AS.GetCore());
+
   // Various flavors of invalid triples.
   AS = ArchSpec();
   EXPECT_FALSE(AS.SetTriple("unknown-unknown-unknown"));
Index: lldb/test/API/functionalities/gdb_remote_client/msp430.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/msp430.yaml
@@ -0,0 +1,426 @@
+# File test.c, compiled with flags "-O0 -g"
+# Source code:
+#
+# int foo = 0;
+#
+# int func() {
+# foo = 1234;
+# return foo;
+# }
+#
+# int main() {
+# return func();
+# }
+#
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  OSABI:   ELFOSABI_STANDALONE
+  Type:ET_EXEC
+  Machine: EM_MSP430
+  Flags:   [  ]
+  Entry:   0x500
+ProgramHeaders:
+  - Type:PT_LOAD
+Flags:   [ PF_X, PF_R ]
+FirstSec:.text
+LastSec: .bss
+VAddr:   0x46C
+Align:   0x4
+  - Type:PT_LOAD
+Flags:   [ PF_W, PF_R ]
+FirstSec:.data
+LastSec: .bss
+VAddr:   0x53C
+Align:   0x4
+  - Type:PT_LOAD
+Flags:   [ PF_R ]
+FirstSec:__interrupt_vector_31
+LastSec: __interrupt_vector_31
+VAddr:   0xFFFE
+Align:   0x4
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+Address: 0x500
+AddressAlign:0x4
+Content: 3140C0FF0C43B0121C05B0128101B240D2043C051C423C053041318002008143B01210053150020030411C4330413C402A0030410C433041
+  - Name:.data
+Type:SHT_PROGBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x53C
+AddressAlign:0x1
+  - Name:.bss
+Type:SHT_NOBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x53C
+AddressAlign:0x2
+Size:0x2
+  - Name:__interrupt_vector_31
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC ]
+Address: 0xFFFE
+AddressAlign:0x1
+Offset:  0xD2
+Content: '0005'
+  - Name:.rodata
+Type:SHT_PROGBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x500
+AddressAlign:0x1
+  - Name:.rodata2
+Type:SHT_PROGBITS
+Flags:   [ SHF_WRITE ]
+Address: 0x500
+AddressAlign:0x1
+  - Name:.noinit
+Type:SHT_PROGBITS
+Flags:   [ SHF_WRITE ]
+Address: 0x53E
+AddressAlign:0x1
+  - Name:.persistent
+Type:SHT_PROGBITS
+Flags:   [ SHF_WRITE ]
+Address: 0x53E
+AddressAlign:0x1
+  - Name:.MSP430.attributes
+Type:SHT_MSP430_ATTRIBUTES
+AddressAlign:0x1
+Content: 4116006D737061626900010B00040106010801
+  - Name:.comment
+Type:SHT_PROGBITS
+Flags:   [ SHF_MERGE, SHF_STRINGS ]
+AddressAlign:0x1
+EntSize:

[Lldb-commits] [PATCH] D148099: [lldb] Reduce chances of spurious failures in some build setups

2023-04-12 Thread Alexander Kornienko via Phabricator via lldb-commits 
alexfh added a comment.

In D148099#4260897 , @DavidSpickett 
wrote:

> Looks good to me.

Thanks for the prompt and helpful review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148099

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

[Lldb-commits] [PATCH] D148099: [lldb] Reduce chances of spurious failures in some build setups

2023-04-12 Thread Alexander Kornienko 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 rGca7a20df1082: [lldb] Reduce chances of spurious failures in 
some build setups (authored by alexfh).

Changed prior to commit:
  https://reviews.llvm.org/D148099?vs=512748=512767#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148099

Files:
  lldb/test/Shell/Commands/command-stop-hook-no-target.test


Index: lldb/test/Shell/Commands/command-stop-hook-no-target.test
===
--- lldb/test/Shell/Commands/command-stop-hook-no-target.test
+++ lldb/test/Shell/Commands/command-stop-hook-no-target.test
@@ -1,4 +1,5 @@
 # RUN: %clang_host -g %S/Inputs/main.c -o %t
 # RUN: %lldb -b -o 'target stop-hook add --name test --shlib test -o 
"expression 95000 + 126"' -o 'file %t' -o 'b main' -o 'r' 2>&1 | FileCheck %s
 # CHECK: Stop hook #1 added
-# CHECK-NOT: 95126
+# CHECK-NOT: (lldb) expression 95000 + 126
+# CHECK-NOT: (int) $0 = 95126


Index: lldb/test/Shell/Commands/command-stop-hook-no-target.test
===
--- lldb/test/Shell/Commands/command-stop-hook-no-target.test
+++ lldb/test/Shell/Commands/command-stop-hook-no-target.test
@@ -1,4 +1,5 @@
 # RUN: %clang_host -g %S/Inputs/main.c -o %t
 # RUN: %lldb -b -o 'target stop-hook add --name test --shlib test -o "expression 95000 + 126"' -o 'file %t' -o 'b main' -o 'r' 2>&1 | FileCheck %s
 # CHECK: Stop hook #1 added
-# CHECK-NOT: 95126
+# CHECK-NOT: (lldb) expression 95000 + 126
+# CHECK-NOT: (int) $0 = 95126
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ca7a20d - [lldb] Reduce chances of spurious failures in some build setups

2023-04-12 Thread Alexander Kornienko via lldb-commits 

Author: Alexander Kornienko
Date: 2023-04-12T13:05:39+02:00
New Revision: ca7a20df108290ed9cd0ceb3137b253c8256a861

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

LOG: [lldb] Reduce chances of spurious failures in some build setups

The test may fail when running from a directory that contains the string used in
CHECK-NOT. We observe flakiness rate of around 3/10. Increasing the length
helps reducing the rate of failures.

Reviewed By: DavidSpickett

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

Added: 


Modified: 
lldb/test/Shell/Commands/command-stop-hook-no-target.test

Removed: 




diff  --git a/lldb/test/Shell/Commands/command-stop-hook-no-target.test 
b/lldb/test/Shell/Commands/command-stop-hook-no-target.test
index cc5e71dff00cd..70e1332091cde 100644
--- a/lldb/test/Shell/Commands/command-stop-hook-no-target.test
+++ b/lldb/test/Shell/Commands/command-stop-hook-no-target.test
@@ -1,4 +1,5 @@
 # RUN: %clang_host -g %S/Inputs/main.c -o %t
 # RUN: %lldb -b -o 'target stop-hook add --name test --shlib test -o 
"expression 95000 + 126"' -o 'file %t' -o 'b main' -o 'r' 2>&1 | FileCheck %s
 # CHECK: Stop hook #1 added
-# CHECK-NOT: 95126
+# CHECK-NOT: (lldb) expression 95000 + 126
+# CHECK-NOT: (int) $0 = 95126



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

[Lldb-commits] [PATCH] D148099: [lldb] Reduce chances of spurious failures in some build setups

2023-04-12 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a reviewer: DavidSpickett.
DavidSpickett accepted this revision.
DavidSpickett added a comment.
This revision is now accepted and ready to land.

Looks good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148099

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

[Lldb-commits] [PATCH] D148099: [lldb] Reduce chances of spurious failures in some build setups

2023-04-12 Thread Alexander Kornienko via Phabricator via lldb-commits 
alexfh updated this revision to Diff 512748.
alexfh added a comment.

Use full expressions in the CHECK-NOT.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148099

Files:
  lldb/test/Shell/Commands/command-stop-hook-no-target.test


Index: lldb/test/Shell/Commands/command-stop-hook-no-target.test
===
--- lldb/test/Shell/Commands/command-stop-hook-no-target.test
+++ lldb/test/Shell/Commands/command-stop-hook-no-target.test
@@ -1,4 +1,5 @@
 # RUN: %clang_host -g %S/Inputs/main.c -o %t
-# RUN: %lldb -b -o 'target stop-hook add --name test --shlib test -o 
"expression 473895000 + 126"' -o 'file %t' -o 'b main' -o 'r' 2>&1 | FileCheck 
%s
+# RUN: %lldb -b -o 'target stop-hook add --name test --shlib test -o 
"expression 95000 + 126"' -o 'file %t' -o 'b main' -o 'r' 2>&1 | FileCheck %s
 # CHECK: Stop hook #1 added
-# CHECK-NOT: 473895126
+# CHECK-NOT: (lldb) expression 95000 + 126
+# CHECK-NOT: (int) $0 = 95126


Index: lldb/test/Shell/Commands/command-stop-hook-no-target.test
===
--- lldb/test/Shell/Commands/command-stop-hook-no-target.test
+++ lldb/test/Shell/Commands/command-stop-hook-no-target.test
@@ -1,4 +1,5 @@
 # RUN: %clang_host -g %S/Inputs/main.c -o %t
-# RUN: %lldb -b -o 'target stop-hook add --name test --shlib test -o "expression 473895000 + 126"' -o 'file %t' -o 'b main' -o 'r' 2>&1 | FileCheck %s
+# RUN: %lldb -b -o 'target stop-hook add --name test --shlib test -o "expression 95000 + 126"' -o 'file %t' -o 'b main' -o 'r' 2>&1 | FileCheck %s
 # CHECK: Stop hook #1 added
-# CHECK-NOT: 473895126
+# CHECK-NOT: (lldb) expression 95000 + 126
+# CHECK-NOT: (int) $0 = 95126
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D148099: [lldb] Reduce chances of spurious failures in some build setups

2023-04-12 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

Could you look for the full output line instead?

  (lldb) expression 1+1
  (int) $0 = 2

Even if `$0` changes maybe `= ` can be checked.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148099

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

[Lldb-commits] [PATCH] D148099: [lldb] Reduce chances of spurious failures in some build setups

2023-04-12 Thread Alexander Kornienko via Phabricator via lldb-commits 
alexfh created this revision.
alexfh added reviewers: JDevlieghere, rupprecht, brooksmoses.
Herald added a project: All.
alexfh requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The test may fail when running from a directory that contains the string used in
CHECK-NOT. We observe flakiness rate of around 3/10. Increasing the length
helps reducing the rate of failures.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148099

Files:
  lldb/test/Shell/Commands/command-stop-hook-no-target.test


Index: lldb/test/Shell/Commands/command-stop-hook-no-target.test
===
--- lldb/test/Shell/Commands/command-stop-hook-no-target.test
+++ lldb/test/Shell/Commands/command-stop-hook-no-target.test
@@ -1,4 +1,4 @@
 # RUN: %clang_host -g %S/Inputs/main.c -o %t
-# RUN: %lldb -b -o 'target stop-hook add --name test --shlib test -o 
"expression 95000 + 126"' -o 'file %t' -o 'b main' -o 'r' 2>&1 | FileCheck %s
+# RUN: %lldb -b -o 'target stop-hook add --name test --shlib test -o 
"expression 473895000 + 126"' -o 'file %t' -o 'b main' -o 'r' 2>&1 | FileCheck 
%s
 # CHECK: Stop hook #1 added
-# CHECK-NOT: 95126
+# CHECK-NOT: 473895126


Index: lldb/test/Shell/Commands/command-stop-hook-no-target.test
===
--- lldb/test/Shell/Commands/command-stop-hook-no-target.test
+++ lldb/test/Shell/Commands/command-stop-hook-no-target.test
@@ -1,4 +1,4 @@
 # RUN: %clang_host -g %S/Inputs/main.c -o %t
-# RUN: %lldb -b -o 'target stop-hook add --name test --shlib test -o "expression 95000 + 126"' -o 'file %t' -o 'b main' -o 'r' 2>&1 | FileCheck %s
+# RUN: %lldb -b -o 'target stop-hook add --name test --shlib test -o "expression 473895000 + 126"' -o 'file %t' -o 'b main' -o 'r' 2>&1 | FileCheck %s
 # CHECK: Stop hook #1 added
-# CHECK-NOT: 95126
+# CHECK-NOT: 473895126
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

2023-04-12 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett updated this revision to Diff 512710.
DavidSpickett added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145580

Files:
  lldb/include/lldb/Core/DumpRegisterValue.h
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
  lldb/include/lldb/Target/RegisterFlags.h
  lldb/include/lldb/Target/RegisterTypeBuilder.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/lldb-forward.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/source/Core/DumpRegisterValue.cpp
  lldb/source/Core/PluginManager.cpp
  lldb/source/DataFormatters/DumpValueObjectOptions.cpp
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Plugins/CMakeLists.txt
  lldb/source/Plugins/RegisterTypeBuilder/CMakeLists.txt
  lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
  lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
  lldb/source/Target/Target.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
  lldb/unittests/Target/RegisterFlagsTest.cpp
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -246,6 +246,12 @@
 * LLDB is now able to show the subtype of signals found in a core file. For example
   memory tagging specific segfaults such as ``SIGSEGV: sync tag check fault``.
 
+* LLDB can now display register fields if they are described in target XML sent
+  by a debug server such as ``gdbserver`` (``lldb-server`` does not currently produce
+  this information). Fields are only printed when reading named registers, for
+  example ``register read cpsr``. They are not shown when reading a register set,
+  ``register read -s 0``.
+
 Changes to Sanitizers
 -
 * For Darwin users that override weak symbols, note that the dynamic linker will
Index: lldb/unittests/Target/RegisterFlagsTest.cpp
===
--- lldb/unittests/Target/RegisterFlagsTest.cpp
+++ lldb/unittests/Target/RegisterFlagsTest.cpp
@@ -121,3 +121,19 @@
 make_field(20, 21), make_field(12, 19), make_field(8, 11),
 make_field(0, 7)});
 }
+
+TEST(RegisterFieldsTest, ReverseFieldOrder) {
+  // Unchanged
+  RegisterFlags rf("", 4, {make_field(0, 31)});
+  ASSERT_EQ(0x12345678ULL, rf.ReverseFieldOrder(0x12345678));
+
+  // Swap the two halves around.
+  RegisterFlags rf2("", 4, {make_field(16, 31), make_field(0, 15)});
+  ASSERT_EQ(0x56781234ULL, rf2.ReverseFieldOrder(0x12345678));
+
+  // Many small fields.
+  RegisterFlags rf3("", 4,
+{make_field(31, 31), make_field(30, 30), make_field(29, 29),
+ make_field(28, 28)});
+  ASSERT_EQ(0x0005ULL, rf3.ReverseFieldOrder(0xA000));
+}
Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -0,0 +1,497 @@
+""" Check that register fields found in target XML are properly processed.
+
+These tests make XML out of string substitution. This can lead to some strange
+failures. Check that the final XML is valid and each child is indented more than
+the parent tag.
+"""
+
+from textwrap import dedent
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+class MultiDocResponder(MockGDBServerResponder):
+# docs is a dictionary of filename -> file content.
+def __init__(self, docs):
+super().__init__()
+self.docs = docs
+
+def qXferRead(self, obj, annex, offset, length):
+try:
+return self.docs[annex], False
+except KeyError:
+return None,
+
+def readRegister(self, regnum):
+return "E01"
+
+def readRegisters(self):
+return ''.join([
+  # Data for all registers requested by the tests below.
+  # 0x7 and 0xE are used because their lsb and msb are opposites, which
+  # is needed for a byte order test.
+  '', # 64 bit x0/r0
+  '', # 32 bit cpsr/fpc
+  '', # 64 bit pc/pswa
+])
+
+class TestXMLRegisterFlags(GDBRemoteTestBase):
+def setup_multidoc_test(self, docs):
+self.server.responder = MultiDocResponder(docs)
+target = self.dbg.CreateTarget('')
+
+if self.TraceOn():
+self.runCmd("log enable gdb-remote packets process")
+self.addTearDownHook(
+lambda: self.runCmd("log disable gdb-remote packets

[Lldb-commits] [PATCH] D145574: [lldb] Read register fields from target XML

2023-04-12 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett updated this revision to Diff 512702.
DavidSpickett added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145574

Files:
  lldb/include/lldb/Target/DynamicRegisterInfo.h
  lldb/include/lldb/lldb-private-types.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/DynamicRegisterInfo.cpp

Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -407,7 +407,7 @@
   {reg.regnum_ehframe, reg.regnum_dwarf, reg.regnum_generic,
reg.regnum_remote, local_regnum},
   // value_regs and invalidate_regs are filled by Finalize()
-  nullptr, nullptr, nullptr
+  nullptr, nullptr, reg.flags_type
 };
 
 m_regs.push_back(reg_info);
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
@@ -38,6 +38,7 @@
 #include "GDBRemoteRegisterContext.h"
 
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringMap.h"
 
 namespace lldb_private {
 namespace repro {
@@ -466,6 +467,15 @@
   // fork helpers
   void DidForkSwitchSoftwareBreakpoints(bool enable);
   void DidForkSwitchHardwareTraps(bool enable);
+
+  // Lists of register fields generated from the remote's target XML.
+  // Pointers to these RegisterFlags will be set in the register info passed
+  // back to the upper levels of lldb. Doing so is safe because this class will
+  // live at least as long as the debug session. We therefore do not store the
+  // data directly in the map because the map may reallocate it's storage as new
+  // entries are added. Which would invalidate any pointers set in the register
+  // info up to that point.
+  llvm::StringMap> m_registers_flags_types;
 };
 
 } // namespace process_gdb_remote
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
@@ -53,6 +53,7 @@
 #include "lldb/Target/ABI.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/MemoryRegionInfo.h"
+#include "lldb/Target/RegisterFlags.h"
 #include "lldb/Target/SystemRuntime.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/TargetList.h"
@@ -84,6 +85,7 @@
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/FormatAdapters.h"
 #include "llvm/Support/Threading.h"
@@ -4029,15 +4031,213 @@
   RegisterSetMap reg_set_map;
 };
 
-bool ParseRegisters(XMLNode feature_node, GdbServerTargetInfo _info,
-std::vector ) {
+static std::vector ParseFlagsFields(XMLNode flags_node,
+  unsigned size) {
+  Log *log(GetLog(GDBRLog::Process));
+  const unsigned max_start_bit = size * 8 - 1;
+
+  // Process the fields of this set of flags.
+  std::vector fields;
+  flags_node.ForEachChildElementWithName("field", [, max_start_bit,
+   ](const XMLNode
+ _node) {
+std::optional name;
+std::optional start;
+std::optional end;
+
+field_node.ForEachAttribute([, , , max_start_bit,
+ ](const llvm::StringRef _name,
+   const llvm::StringRef _value) {
+  // Note that XML in general requires that each of these attributes only
+  // appears once, so we don't have to handle that here.
+  if (attr_name == "name") {
+LLDB_LOG(log,
+ "ProcessGDBRemote::ParseFlags Found field node name \"{0}\"",
+ attr_value.data());
+name = attr_value;
+  } else if (attr_name == "start") {
+unsigned parsed_start = 0;
+if (llvm::to_integer(attr_value, parsed_start)) {
+  if (parsed_start > max_start_bit) {
+LLDB_LOG(
+log,
+"ProcessGDBRemote::ParseFlags Invalid start {0} in field node, "
+"cannot be > {1}",
+parsed_start, max_start_bit);
+  } else
+start = parsed_start;
+} else {
+  LLDB_LOG(log,
+   "ProcessGDBRemote::ParseFlags Invalid start \"{0}\" in "
+   "field node",
+   attr_value.data());
+}
+  } else if (attr_name == "end") {
+unsigned

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

2023-04-12 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett updated this revision to Diff 512692.
DavidSpickett added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145566

Files:
  lldb/include/lldb/Target/RegisterFlags.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/RegisterFlags.cpp
  lldb/unittests/Target/CMakeLists.txt
  lldb/unittests/Target/RegisterFlagsTest.cpp

Index: lldb/unittests/Target/RegisterFlagsTest.cpp
===
--- /dev/null
+++ lldb/unittests/Target/RegisterFlagsTest.cpp
@@ -0,0 +1,123 @@
+//===-- RegisterFlagsTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Target/RegisterFlags.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace lldb;
+
+TEST(RegisterFlagsTest, Field) {
+  // We assume that start <= end is always true, so that is not tested here.
+
+  RegisterFlags::Field f1("abc", 0, 0);
+  ASSERT_EQ(f1.GetName(), "abc");
+  // start == end means a 1 bit field.
+  ASSERT_EQ(f1.GetSizeInBits(), (unsigned)1);
+  ASSERT_EQ(f1.GetMask(), (uint64_t)1);
+  ASSERT_EQ(f1.GetValue(0), (uint64_t)0);
+  ASSERT_EQ(f1.GetValue(3), (uint64_t)1);
+
+  // End is inclusive meaning that start 0 to end 1 includes bit 1
+  // to make a 2 bit field.
+  RegisterFlags::Field f2("", 0, 1);
+  ASSERT_EQ(f2.GetSizeInBits(), (unsigned)2);
+  ASSERT_EQ(f2.GetMask(), (uint64_t)3);
+  ASSERT_EQ(f2.GetValue(UINT64_MAX), (uint64_t)3);
+  ASSERT_EQ(f2.GetValue(UINT64_MAX & ~(uint64_t)3), (uint64_t)0);
+
+  // If the field doesn't start at 0 we need to shift up/down
+  // to account for it.
+  RegisterFlags::Field f3("", 2, 5);
+  ASSERT_EQ(f3.GetSizeInBits(), (unsigned)4);
+  ASSERT_EQ(f3.GetMask(), (uint64_t)0x3c);
+  ASSERT_EQ(f3.GetValue(UINT64_MAX), (uint64_t)0xf);
+  ASSERT_EQ(f3.GetValue(UINT64_MAX & ~(uint64_t)0x3c), (uint64_t)0);
+
+  // Fields are sorted lowest starting bit first.
+  ASSERT_TRUE(f2 < f3);
+  ASSERT_FALSE(f3 < f1);
+  ASSERT_FALSE(f1 < f2);
+  ASSERT_FALSE(f1 < f1);
+}
+
+static RegisterFlags::Field make_field(unsigned start, unsigned end) {
+  return RegisterFlags::Field("", start, end);
+}
+
+TEST(RegisterFlagsTest, FieldOverlaps) {
+  // Single bit fields
+  ASSERT_FALSE(make_field(0, 0).Overlaps(make_field(1, 1)));
+  ASSERT_TRUE(make_field(1, 1).Overlaps(make_field(1, 1)));
+  ASSERT_FALSE(make_field(1, 1).Overlaps(make_field(3, 3)));
+
+  ASSERT_TRUE(make_field(0, 1).Overlaps(make_field(1, 2)));
+  ASSERT_TRUE(make_field(1, 2).Overlaps(make_field(0, 1)));
+  ASSERT_FALSE(make_field(0, 1).Overlaps(make_field(2, 3)));
+  ASSERT_FALSE(make_field(2, 3).Overlaps(make_field(0, 1)));
+
+  ASSERT_FALSE(make_field(1, 5).Overlaps(make_field(10, 20)));
+  ASSERT_FALSE(make_field(15, 30).Overlaps(make_field(7, 12)));
+}
+
+TEST(RegisterFlagsTest, PaddingDistance) {
+  // We assume that this method is always called with a more significant
+  // (start bit is higher) field first and that they do not overlap.
+
+  // [field 1][field 2]
+  ASSERT_EQ(make_field(1, 1).PaddingDistance(make_field(0, 0)), 0ULL);
+  // [field 1][..][field 2]
+  ASSERT_EQ(make_field(2, 2).PaddingDistance(make_field(0, 0)), 1ULL);
+  // [field 1][field 1][field 2]
+  ASSERT_EQ(make_field(1, 2).PaddingDistance(make_field(0, 0)), 0ULL);
+  // [field 1][30 bits free][field 2]
+  ASSERT_EQ(make_field(31, 31).PaddingDistance(make_field(0, 0)), 30ULL);
+}
+
+static void test_padding(const std::vector ,
+ const std::vector ) {
+  RegisterFlags rf("", 4, fields);
+  EXPECT_THAT(expected, ::testing::ContainerEq(rf.GetFields()));
+}
+
+TEST(RegisterFlagsTest, RegisterFlagsPadding) {
+  // When creating a set of flags we assume that:
+  // * There are >= 1 fields.
+  // * They are sorted in descending order.
+  // * There may be gaps between each field.
+
+  // Needs no padding
+  auto fields =
+  std::vector{make_field(16, 31), make_field(0, 15)};
+  test_padding(fields, fields);
+
+  // Needs padding in between the fields, single bit.
+  test_padding({make_field(17, 31), make_field(0, 15)},
+   {make_field(17, 31), make_field(16, 16), make_field(0, 15)});
+  // Multiple bits of padding.
+  test_padding({make_field(17, 31), make_field(0, 14)},
+   {make_field(17, 31), make_field(15, 16), make_field(0, 14)});
+
+  // Padding before first field, single bit.
+  test_padding({make_field(0, 30)}, {make_field(31, 31), make_field(0, 30)});
+  // Multiple bits.
+  test_padding({make_field(0, 15)}, {make_field(16, 31), make_field(0, 15)});
+
+  // Padding after last field, single bit.
+

[Lldb-commits] [PATCH] D147820: debugserver: move AArch64 watchpoint traps within a watchpointed region, parse ESR flags and send them to lldb

2023-04-12 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added inline comments.



Comment at: lldb/test/API/commands/watchpoints/unaligned-watchpoint/main.c:8-11
+   printf("%lld %lld\n", (*u64_p)++, (*u64_p)++);
+   printf("%lld %lld\n", (*u64_p)++, (*u64_p)++);
+   printf("%lld %lld\n", (*u64_p)++, (*u64_p)++);
+   printf("%lld %lld\n", (*u64_p)++, (*u64_p)++);

jasonmolenda wrote:
> DavidSpickett wrote:
> > These do what exactly?
> These are doing 64-bit writes to `u64_p` which is pointing to the start of 
> the `uint8_t buf[8]`, while I'm watching 1 byte in the middle, so the FAR 
> address I get is the start of the buffer (and outside of the address range 
> I'm watching).  This is the one that lldb currently skips over silently on 
> AArch64 Linux/FreeBSD.  Doing two of these writes on each source line was 
> something I did while testing something else, I didn't really need to write 
> it this way for this test case.
So can we have the minimum writes here then?

I guess that another `*u64_p = 5;` would be enough.



Comment at: lldb/tools/debugserver/source/DNBDefs.h:284-297
+  bool esr_fields_set;
+  struct {
+uint32_t
+iss; // "ISS encoding for an exception from a Watchpoint exception"
+uint32_t wpt;  // Watchpoint number
+bool wptv; // Watchpoint number Valid
+bool wpf;  // Watchpoint might be false-positive

JDevlieghere wrote:
> Instead of a boolean and a field, you could make it a 
> `std::optional`. 
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147820

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

[Lldb-commits] [PATCH] D147816: Clarify how watchpoint description in stop packets work, fix AArch64 unintended behavior

2023-04-12 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid accepted this revision.
omjavaid added a comment.
This revision is now accepted and ready to land.

Looks good to me as well


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147816

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

[Lldb-commits] [PATCH] D147816: Clarify how watchpoint description in stop packets work, fix AArch64 unintended behavior

2023-04-12 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

Looks good to me, @omjavaid ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147816

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