[Lldb-commits] [PATCH] D107669: [trace] [intel pt] Create a "process trace save" command

[walter erquinigo via Phabricator via lldb-commits]

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

discussed offline of some improvements to make


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

https://reviews.llvm.org/D107669

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

[Lldb-commits] [PATCH] D107674: [tests] [trace] Add a more comprehensive test for `thread trace export ctf` command

[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 rGef28c78350db: [tests] [trace] Add a more comprehensive test 
for `thread trace export ctf`… (authored by Walter Erquinigo 
wall...@fb.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107674

Files:
  lldb/docs/htr.rst
  lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
  lldb/source/Plugins/TraceExporter/common/TraceHTR.h
  lldb/source/Plugins/TraceExporter/docs/htr.rst
  lldb/test/API/commands/trace/TestTraceExport.py

Index: lldb/test/API/commands/trace/TestTraceExport.py
===
--- lldb/test/API/commands/trace/TestTraceExport.py
+++ lldb/test/API/commands/trace/TestTraceExport.py
@@ -34,22 +34,96 @@
 substrs=["error: Process is not being traced"],
 error=True)
 
-def testExportCreatesFile(self):
+
+def testHtrBasicSuperBlockPassFullCheck(self):
+'''
+Test the BasicSuperBlock pass of HTR.
+
+This test uses a very small trace so that the expected output is digestible and
+it's possible to manually verify the behavior of the algorithm.
+
+This test exhaustively checks that each entry
+in the output JSON is equal to the expected value.
+
+'''
+
 self.expect("trace load -v " +
 os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
 substrs=["intel-pt"])
 
 ctf_test_file = self.getBuildArtifact("ctf-test.json")
 
-if os.path.exists(ctf_test_file):
-remove_file(ctf_test_file)
 self.expect(f"thread trace export ctf --file {ctf_test_file}")
 self.assertTrue(os.path.exists(ctf_test_file))
 
+with open(ctf_test_file) as f:
+data = json.load(f)
 
-def testHtrBasicSuperBlockPass(self):
 '''
-Test the BasicSuperBlock pass of HTR
+The expected JSON contained by "ctf-test.json"
+
+dur: number of instructions in the block
+
+name: load address of the first instruction of the block and the
+name of the most frequently called function from the block (if applicable)
+
+ph: 'X' for Complete events (see link to documentation below)
+
+pid: the ID of the HTR layer the blocks belong to
+
+ts: offset from the beginning of the trace for the first instruction in the block
+
+See https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview#heading=h.j75x71ritcoy
+for documentation on the Trace Event Format
+'''
+# Comments on the right indicate if a block is a "head" and/or "tail"
+# See BasicSuperBlockMerge in TraceHTR.h for a description of the algorithm
+expected = [
+{"dur":1,"name":"0x400511","ph":"X","pid":0,"ts":0},
+{"dur":1,"name":"0x400518","ph":"X","pid":0,"ts":1},
+{"dur":1,"name":"0x40051f","ph":"X","pid":0,"ts":2},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":3}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":4}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":5},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":6},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":7}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":8}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":9},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":10},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":11}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":12}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":13},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":14},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":15}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":16}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":17},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":18},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":19}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":20}, # tail
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":3}},"dur":3,"name":"0x400511","ph":"X","pid":1,"ts":0},
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400529","ph":"X","pid":1,"ts":3}, # head, tail
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400521","ph":"X","pid":1,"ts":5},
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400529","ph":"X","pid":1,"ts":7}, # head, tail
+

[Lldb-commits] [lldb] ef28c78 - [tests] [trace] Add a more comprehensive test for `thread trace export ctf` command

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2021-08-11T20:50:10-07:00
New Revision: ef28c78350dbf2a5dff23ac7e5f2dc0a7c24e184

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

LOG: [tests] [trace] Add a more comprehensive test for `thread trace export 
ctf` command

Follow up on https://reviews.llvm.org/D105741

- Add new test that exhaustively checks the output file's content
- Fix typos in documentation and other minor fixes

Reviewed By: wallace

Original Author: jj10306

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

Added: 
lldb/source/Plugins/TraceExporter/docs/htr.rst

Modified: 
lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
lldb/source/Plugins/TraceExporter/common/TraceHTR.h
lldb/test/API/commands/trace/TestTraceExport.py

Removed: 
lldb/docs/htr.rst



diff  --git a/lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp 
b/lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
index 2f0a3540f31e6..d29445cc004f4 100644
--- a/lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
+++ b/lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
@@ -112,7 +112,7 @@ HTRBlockMetadata 
HTRInstructionLayer::GetMetadataByIndex(size_t index) const {
   func_calls[*func_name] = 1;
 }
   }
-  return {instruction_load_address, 1, func_calls};
+  return {instruction_load_address, 1, std::move(func_calls)};
 }
 
 size_t HTRInstructionLayer::GetNumUnits() const {
@@ -391,20 +391,35 @@ llvm::json::Value lldb_private::toJSON(const TraceHTR 
) {
 std::string load_address_hex_string(stream.str());
 display_name.assign(load_address_hex_string);
 
-layers_as_json.emplace_back(llvm::json::Object{
-{"name", display_name},
-{"ph", "B"},
-{"ts", (int64_t)i},
+// name: load address of the first instruction of the block and the name
+// of the most frequently called function from the block (if applicable)
 
-{"pid", (int64_t)layer_id},
-{"tid", (int64_t)layer_id},
-});
+// ph: the event type - 'X' for Complete events (see link to documentation
+// below)
+
+// Since trace timestamps aren't yet supported in HTR, the ts (timestamp) 
is
+// based on the instruction's offset in the trace and the dur (duration) is
+// 1 since this layer contains single instructions. Using the instruction
+// offset and a duration of 1 oversimplifies the true timing information of
+// the trace, nonetheless, these approximate timestamps/durations provide 
an
+// clear visualization of the trace.
+
+// ts: offset from the beginning of the trace for the first instruction in
+// the block
+
+// dur: 1 since this layer contains single instructions.
 
+// pid: the ID of the HTR layer the blocks belong to
+
+// See
+// 
https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview#heading=h.j75x71ritcoy
+// for documentation on the Trace Event Format
 layers_as_json.emplace_back(llvm::json::Object{
-{"ph", "E"},
-{"ts", (int64_t)i + 1},
+{"name", display_name},
+{"ph", "X"},
+{"ts", (int64_t)i},
+{"dur", 1},
 {"pid", (int64_t)layer_id},
-{"tid", (int64_t)layer_id},
 });
   }
 
@@ -420,8 +435,6 @@ llvm::json::Value lldb_private::toJSON(const TraceHTR ) 
{
 
   HTRBlockMetadata metadata = block.GetMetadata();
 
-  size_t end_ts = start_ts + metadata.GetNumInstructions();
-
   llvm::Optional most_freq_func =
   metadata.GetMostFrequentlyCalledFunction();
   std::stringstream stream;
@@ -431,22 +444,23 @@ llvm::json::Value lldb_private::toJSON(const TraceHTR 
) {
   most_freq_func ? offset_hex_string + ": " + most_freq_func->str()
  : offset_hex_string;
 
+  // Since trace timestamps aren't yet supported in HTR, the ts (timestamp)
+  // and dur (duration) are based on the block's offset in the trace and
+  // number of instructions in the block, respectively. Using the block
+  // offset and the number of instructions oversimplifies the true timing
+  // information of the trace, nonetheless, these approximate
+  // timestamps/durations provide an understandable visualization of the
+  // trace.
+  auto duration = metadata.GetNumInstructions();
   layers_as_json.emplace_back(llvm::json::Object{
   {"name", display_name},
-  {"ph", "B"},
+  {"ph", "X"},
   {"ts", (int64_t)start_ts},
+  {"dur", (int64_t)duration},
   {"pid", (int64_t)layer_id},
-  {"tid", (int64_t)layer_id},
-  });
-
-  layers_as_json.emplace_back(llvm::json::Object{
-  {"ph", "E"},
-  {"ts", (int64_t)end_ts},
-  {"pid", (int64_t)layer_id},
-  

[Lldb-commits] [PATCH] D107669: [trace] [intel pt] Create a "process trace save" command

[hanbing wang via Phabricator via lldb-commits]

hanbingwang added inline comments.



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:87
+llvm::Expected
+TraceIntelPTSessionFileSaver::BuildProcessesSection(lldb::ProcessSP 
_sp,
+TraceIntelPT _ipt,

wallace wrote:
> you could move BuildProcessesSection, BuildThreadsSection and 
> BuildModulesSection to TraceSessionSaver.h/cpp if you provide a callback that 
> receives a thread_id and returns a raw trace. Then almost all the code would 
> be reusable by other Trace plug-ins
Sorry I did not make changes here. A bit unsure how to "provide a callback that 
receives a thread_id and returns a raw trace" and how would it work?



Comment at: lldb/test/API/commands/trace/TestTraceSave.py:34-36
+self.expect("trace load -v " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
+substrs=["intel-pt"])

wallace wrote:
> now that we are changing to live processes, this won't work. Instead, trace 
> one of the existing binary files in the test folder, run it, get the first 
> and last 20 instructions, save the contents in strings, then save the trace, 
> then load it and dump the instructions again, finally assert that the outputs 
> are the same
I'm not sure how to "save the contents in strings"?


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

https://reviews.llvm.org/D107669

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

[Lldb-commits] [PATCH] D107669: [trace] [intel pt] Create a "process trace save" command

[hanbing wang via Phabricator via lldb-commits]

hanbingwang updated this revision to Diff 365883.
hanbingwang marked an inline comment as done.
hanbingwang added a comment.

*trace.h: 
-rename SaveToDisk() to SaveLiveTraceToDisk()

*IntelPTDecoder.h, IntelPTDecoder.cpp: 
-removed GetRawTrace()

*TraceIntelPT.h, TraceIntelPT.cpp: 
-removed GetThreadBuffer()
-in function SaveToDisk(), replace "process_sp" with "m_live_process"

*TraceIntelPTSessionFileSaver.h, TraceIntelPTSessionFileSaver.cpp:
-renamed to TraceIntelPTSessionSaver.h and TraceIntelPTSessionSaver.cpp
-the function BuildModulesSection() copies modules inside /modules
-create new constructors for struct JSONTraceIntelPTCPUInfo
-error checking for writing stream to file


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

https://reviews.llvm.org/D107669

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/common/CMakeLists.txt
  lldb/source/Plugins/Trace/common/TraceJSONStructs.cpp
  lldb/source/Plugins/Trace/common/TraceJSONStructs.h
  lldb/source/Plugins/Trace/common/TraceSessionFileParser.cpp
  lldb/source/Plugins/Trace/common/TraceSessionFileParser.h
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.h
  lldb/test/API/commands/trace/TestTraceSave.py

Index: lldb/test/API/commands/trace/TestTraceSave.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceSave.py
@@ -0,0 +1,49 @@
+import lldb
+from intelpt_testcase import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceSave(TraceIntelPTTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def testErrorMessages(self):
+# We first check the output when there are no targets
+self.expect("process trace save",
+substrs=["error: invalid target, create a target using the 'target create' command"],
+error=True)
+
+# We now check the output when there's a non-running target
+self.expect("target create " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+
+self.expect("process trace save",
+substrs=["error: invalid process"],
+error=True)
+
+# Now we check the output when there's a running target without a trace
+self.expect("b main")
+self.expect("run")
+
+self.expect("process trace save",
+substrs=["error: Process is not being traced"],
+error=True)
+
+def testSaveTrace(self):
+self.expect("target create " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+self.expect("b main")
+self.expect("r")
+self.expect("thread trace start")
+self.expect("n")
+self.expect("thread trace dump instructions", substrs=["""0x00400511movl   $0x0, -0x4(%rbp)
+no more data"""])
+# Save the trace to 
+self.expect("process trace save -d " +
+os.path.join(self.getBuildDir(), "intelpt-trace", "trace_copy_dir"))
+# Load the trace just saved
+self.expect("trace load -v " +
+os.path.join(self.getBuildDir(), "intelpt-trace", "trace_copy_dir", "trace.json"),
+substrs=["intel-pt"])
+self.expect("thread trace dump instructions", substrs=["""0x00400511movl   $0x0, -0x4(%rbp)
+no more data"""])
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.h
===
--- /dev/null
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.h
@@ -0,0 +1,140 @@
+//===-- TraceIntelPTSessionSaver.h ---*- C++ //-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPTSESSIONSAVER_H
+#define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPTSESSIONSAVER_H
+
+#include "TraceIntelPT.h"
+
+#include "../common/TraceSessionFileParser.h"
+
+namespace lldb_private {
+namespace trace_intel_pt {
+
+class TraceIntelPT;
+
+class 

[Lldb-commits] [lldb] 7ce739a - Update gdbremote_testcase.py to allow new k-v pair in qMemoryRegionInfo

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2021-08-11T17:34:52-07:00
New Revision: 7ce739a87834785e2ddbc893fc833346413d360f

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

LOG: Update gdbremote_testcase.py to allow new k-v pair in qMemoryRegionInfo

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
index b6243af54350a..c45ef5a3c82c0 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -724,7 +724,8 @@ def parse_memory_region_packet(self, context):
  "flags",
  "name",
  "error",
- "dirty-pages"])
+ "dirty-pages",
+ "type"])
 self.assertIsNotNone(val)
 
 mem_region_dict["name"] = seven.unhexlify(mem_region_dict.get("name", 
""))



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

[Lldb-commits] [lldb] 2b30fc2 - Fix two bugs with stack corefiles patch, restrict test built debugserver

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2021-08-11T17:19:31-07:00
New Revision: 2b30fc2ff3cadd085f7103cb953cd9cf091ee9b9

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

LOG: Fix two bugs with stack corefiles patch, restrict test built debugserver

These two tests, TestSkinnyCorefile.py and TestStackCorefile.py,
require a new debugserver on darwin systems to run correctly; for now,
skip them if the system debugserver is in use.  There's no easy way to
test if the debugserver being used supports either of these memory
region info features. For end users, the fallback will be a full
corefile and that's not the worst thing, but for the tests it is a
problem.

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
lldb/test/API/macosx/stack-corefile/TestStackCorefile.py
lldb/tools/debugserver/source/RNBRemote.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index de494bc08..8d3b3ebbe95f6 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6616,6 +6616,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP 
_sp,
 std::vector combined_page_objects;
 page_object last_obj;
 last_obj.addr = LLDB_INVALID_ADDRESS;
+last_obj.size = 0;
 for (page_object obj : pages_to_copy) {
   if (last_obj.addr == LLDB_INVALID_ADDRESS) {
 last_obj = obj;
@@ -6629,12 +6630,10 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP 
_sp,
   combined_page_objects.push_back(last_obj);
   last_obj = obj;
 }
-
-// If we only ended up with one contiguous memory segment
-if (combined_page_objects.size() == 0 &&
-last_obj.addr != LLDB_INVALID_ADDRESS) {
+// Add the last entry we were looking to combine
+// on to the array.
+if (last_obj.addr != LLDB_INVALID_ADDRESS && last_obj.size != 0)
   combined_page_objects.push_back(last_obj);
-}
 
 for (page_object obj : combined_page_objects) {
   uint32_t cmd_type = LC_SEGMENT_64;

diff  --git a/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py 
b/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
index a0822a6e392d9..4257aa5ca26a5 100644
--- a/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
+++ b/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
@@ -16,6 +16,7 @@ class TestSkinnyCorefile(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
+@skipIfOutOfTreeDebugserver  # newer debugserver required for these 
qMemoryRegionInfo types
 @skipIf(debug_info=no_match(["dsym"]), bugnumber="This test is looking 
explicitly for a dSYM")
 @skipUnlessDarwin
 def test_lc_note(self):

diff  --git a/lldb/test/API/macosx/stack-corefile/TestStackCorefile.py 
b/lldb/test/API/macosx/stack-corefile/TestStackCorefile.py
index a9104e73419dc..b1c0fa98712e3 100644
--- a/lldb/test/API/macosx/stack-corefile/TestStackCorefile.py
+++ b/lldb/test/API/macosx/stack-corefile/TestStackCorefile.py
@@ -13,6 +13,7 @@ class TestStackCorefile(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
+@skipIfOutOfTreeDebugserver  # newer debugserver required for these 
qMemoryRegionInfo types
 @no_debug_info_test
 @skipUnlessDarwin
 def test(self):

diff  --git a/lldb/tools/debugserver/source/RNBRemote.cpp 
b/lldb/tools/debugserver/source/RNBRemote.cpp
index b70c7ae3707e5..dd8b90297995a 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -4311,6 +4311,7 @@ rnb_err_t RNBRemote::HandlePacket_MemoryRegionInfo(const 
char *p) {
 }
 ostrm << ";";
 if (!region_info.vm_types.empty()) {
+  ostrm << "type:";
   for (size_t i = 0; i < region_info.vm_types.size(); i++) {
 if (i)
   ostrm << ",";



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

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D107869#2940428 , @clayborg wrote:

> In D107869#2940016 , @OmarEmaraDev 
> wrote:
>
>> @jingham I wasn't arguing that we should remove those environment variables, 
>> on the contrary. Greg was suggesting that we populate the environment field 
>> with the target environment instead of implicitly adding them to the 
>> environment of the launch info. The problem with that is that there is a 
>> large number of environments that gets added (15 in my case) as shown in the 
>> following screenshot. What I was saying in my comment above is that I don't 
>> think we should show those to the user, we should transparently add them 
>> instead and let the user add additional variables as needed. We can then 
>> look into adding other settings to exclude them if necessary.
>
> I was actually suggesting only priming the env var list with what was 
> contained in the "target.env-vars" setting. This doesn't include the extra 
> env vars that are inherited. Just the ones that the user explicitly set. That 
> defaults to nothing usually. If we go this way, then we should add a new 
> boolean for inheriting the env vars that defaults to the current value of 
> "target.inherit-env"

I think it would be confusing to have a window called "Environment Variables" 
that has a checkbox that says "Inherit Environment" and when I check or uncheck 
that checkbox the "Environment Variables" pane contents doesn't change.  We had 
this same confusion internally and in command-line lldb for quite a while, but 
that was causing confusion and we sorted that all out a year or so ago.  We 
shouldn't replicate this confusion in the UI.

There's some consistency in having a pane in the Launch Parameters that allows 
you to add "User-Specified Environment Variables" as well as a checkbox to 
inherit the environment.  After all, you can't edit the environment variables 
you inherit other than by overriding them or unsetting them with the 
unset-env-vars setting.  So this would be a consistent set of editable 
parameters.  But it needs to be clear what this is actually displaying.

It would still be nice if the UI gave a way to see the resultant target 
environment.  Maybe a "show inherited env-vars" control to go along with the 
"inherit-env" checkbox?  But it's not terrible if I have to go out to the 
console to see that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107869

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

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3223
+
+const Environment _environment = target->GetEnvironment();
+m_enviroment_field->AddEnvironmentVariables(target_environment);

Does this currently get all target env vars including the inherited ones?



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3233
+PlatformSP platform = target->GetPlatform();
+return platform->GetWorkingDirectory().GetCString();
+  }

Use FileSpec::GetPath(), it already returns a std::string. The current code 
could crash if it returns NULL



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3270
+  launch_info.GetArguments().AppendArgument(target_settings_argv0);
+  launch_info.SetExecutableFile(executable_module->GetPlatformFileSpec(),
+false);

They get delivered to the Process class itself. We can later make a window for 
process STDIO if we need to. But the STDIO will live inside the 
lldb_private::Process class and get stored. It can be fetched with 
Process::GetSTDOUT(...) and Process::GetSTDERR(...)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107869

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

[Lldb-commits] [lldb] f899445 - [lldb] Fix TestFormattersBoolRefPtr on AS

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-08-11T14:55:39-07:00
New Revision: f89944530726f7b315b30670a7e1f93d0cd926f0

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

LOG: [lldb] Fix TestFormattersBoolRefPtr on AS

BOOL is bool instead of signed char on ARM. See
https://reviews.llvm.org/D93421#inline-874116 for details.

Added: 


Modified: 

lldb/test/API/functionalities/data-formatter/boolreference/TestFormattersBoolRefPtr.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/data-formatter/boolreference/TestFormattersBoolRefPtr.py
 
b/lldb/test/API/functionalities/data-formatter/boolreference/TestFormattersBoolRefPtr.py
index 815a8ab903c07..95b3b5ce24dc5 100644
--- 
a/lldb/test/API/functionalities/data-formatter/boolreference/TestFormattersBoolRefPtr.py
+++ 
b/lldb/test/API/functionalities/data-formatter/boolreference/TestFormattersBoolRefPtr.py
@@ -76,11 +76,14 @@ def cleanup():
 if not(isArm):
 self.expect('frame variable unset', substrs=['12'])
 
+# BOOL is bool instead of signed char on ARM.
+converted_YES = "-1" if not isArm else "YES"
+
 self.expect_expr('myField', result_type="BoolBitFields",
  result_children=[
  ValueCheck(name="fieldOne", summary="NO"),
- ValueCheck(name="fieldTwo", summary="-1"),
+ ValueCheck(name="fieldTwo", summary=converted_YES),
  ValueCheck(name="fieldThree", summary="NO"),
  ValueCheck(name="fieldFour", summary="NO"),
- ValueCheck(name="fieldFive", summary="-1")
+ ValueCheck(name="fieldFive", summary=converted_YES)
  ])



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

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D107869#2940016 , @OmarEmaraDev 
wrote:

> @jingham I wasn't arguing that we should remove those environment variables, 
> on the contrary. Greg was suggesting that we populate the environment field 
> with the target environment instead of implicitly adding them to the 
> environment of the launch info. The problem with that is that there is a 
> large number of environments that gets added (15 in my case) as shown in the 
> following screenshot. What I was saying in my comment above is that I don't 
> think we should show those to the user, we should transparently add them 
> instead and let the user add additional variables as needed. We can then look 
> into adding other settings to exclude them if necessary.

I was actually suggesting only priming the env var list with what was contained 
in the "target.env-vars" setting. This doesn't include the extra env vars that 
are inherited. Just the ones that the user explicitly set. That defaults to 
nothing usually. If we go this way, then we should add a new boolean for 
inheriting the env vars that defaults to the current value of 
"target.inherit-env"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107869

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

[Lldb-commits] [lldb] 9f4b130 - [lldb] Replace assertTrue(foo in bar) with assertIn(foo, bar)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-08-11T14:40:21-07:00
New Revision: 9f4b130defc17cbb107ca2186ef6b0f0ff649fd3

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

LOG: [lldb] Replace assertTrue(foo in bar) with assertIn(foo, bar)

The benefit of using assertIn is an improved error message when the
assertion fails:

  AssertionError: False is not True

becomes

  AssertionError: 'have ints 5 20 20 5' not found in '""'

Added: 


Modified: 
lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py

Removed: 




diff  --git a/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py 
b/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
index d313bb9a4f44a..a0822a6e392d9 100644
--- a/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
+++ b/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
@@ -140,7 +140,7 @@ def test_lc_note(self):
 self.assertEqual(to_be_removed_dirty_data.GetValueAsUnsigned(), 20)
 
 present_heap_buf = f0.FindVariable("present_heap_buf")
-self.assertTrue("have ints 5 20 20 5" in present_heap_buf.GetSummary())
+self.assertIn("have ints 5 20 20 5", present_heap_buf.GetSummary())
 
 
 # f1 is to_be_removed() in libto-be-removed.dylib



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

[Lldb-commits] [lldb] 3f96438 - [lldb] Skip TestConcurrent.* watchpoint tests for Darwin on ARM

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-08-11T14:26:31-07:00
New Revision: 3f96438c201e933809fabc39368686884b98f057

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

LOG: [lldb] Skip TestConcurrent.* watchpoint tests for Darwin on ARM

All TestConcurrent.* tests that involve watchpoints are broken on
ARM-based Darwin platforms, including Apple Silicon.

rdar://81811539

Added: 


Modified: 

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatchBreak.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py
 
b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py
index 9404cb505d6c7..18f6f12907fe0 100644
--- 
a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py
+++ 
b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py
@@ -13,6 +13,10 @@ class ConcurrentDelayWatchBreak(ConcurrentEventsBase):
 
 # Atomic sequences are not supported yet for MIPS in LLDB.
 @skipIf(triple='^mips')
+@skipIf(
+oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
+archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
+bugnumber="rdar://81811539")
 @add_test_categories(["watchpoint"])
 def test(self):
 """Test (1-second delay) watchpoint and a breakpoint in multiple 
threads."""

diff  --git 
a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
 
b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
index 2274fbb6d4a64..79a5c3e90f2cb 100644
--- 
a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
+++ 
b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
@@ -1,4 +1,3 @@
-
 import unittest2
 
 from lldbsuite.test.decorators import *
@@ -13,6 +12,10 @@ class ConcurrentManyWatchpoints(ConcurrentEventsBase):
 
 # Atomic sequences are not supported yet for MIPS in LLDB.
 @skipIf(triple='^mips')
+@skipIf(
+oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
+archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
+bugnumber="rdar://81811539")
 @add_test_categories(["watchpoint"])
 @skipIfOutOfTreeDebugserver
 def test(self):

diff  --git 
a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py
 
b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py
index 8af1b41dbf6e1..ab11cae8819b2 100644
--- 
a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py
+++ 
b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py
@@ -15,6 +15,10 @@ class ConcurrentNWatchNBreak(ConcurrentEventsBase):
 @skipIf(triple='^mips')
 @expectedFailureAll(archs=["aarch64"], oslist=["freebsd"],
 bugnumber="llvm.org/pr49433")
+@skipIf(
+oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
+archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
+bugnumber="rdar://81811539")
 @add_test_categories(["watchpoint"])
 def test(self):
 """Test with 5 watchpoint and breakpoint threads."""

diff  --git 
a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py
 
b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py
index 9624e10eb0f54..fa2e022304fe6 100644
--- 
a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py
+++ 
b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py
@@ -16,6 +16,10 @@ class ConcurrentSignalNWatchNBreak(ConcurrentEventsBase):
 @expectedFailureNetBSD
 @expectedFailureAll(archs=["aarch64"], 

[Lldb-commits] [lldb] b97afc9 - [lldb] Update MemoryRegionInfo ctors in unit tests

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-08-11T14:13:41-07:00
New Revision: b97afc9dc0e96116b1369e4dbbcf9ada698b8802

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

LOG: [lldb] Update MemoryRegionInfo ctors in unit tests

Added: 


Modified: 
lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
lldb/unittests/Process/minidump/MinidumpParserTest.cpp

Removed: 




diff  --git a/lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp 
b/lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
index 66db61ff65969..bc714e26fbfdf 100644
--- a/lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
+++ b/lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
@@ -91,6 +91,7 @@ INSTANTIATE_TEST_SUITE_P(
  MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
  MemoryRegionInfo::eYes, ConstString("[abc]"),
  MemoryRegionInfo::eDontKnow, 0,
+ MemoryRegionInfo::eDontKnow,
  MemoryRegionInfo::eDontKnow),
 },
 "unexpected /proc/{pid}/maps exec permission char"),
@@ -98,12 +99,12 @@ INSTANTIATE_TEST_SUITE_P(
 std::make_tuple(
 "55a4512f7000-55a451b68000 rw-p  00:00 0[heap]",
 MemoryRegionInfos{
-MemoryRegionInfo(make_range(0x55a4512f7000, 0x55a451b68000),
- MemoryRegionInfo::eYes, 
MemoryRegionInfo::eYes,
- MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
- ConstString("[heap]"),
- MemoryRegionInfo::eDontKnow, 0,
- MemoryRegionInfo::eDontKnow),
+MemoryRegionInfo(
+make_range(0x55a4512f7000, 0x55a451b68000),
+MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
+MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+ConstString("[heap]"), MemoryRegionInfo::eDontKnow, 0,
+MemoryRegionInfo::eDontKnow, MemoryRegionInfo::eDontKnow),
 },
 ""),
 // Multiple entries
@@ -112,18 +113,18 @@ INSTANTIATE_TEST_SUITE_P(
 "ff60-ff601000 r-xp  00:00 0 "
 "[vsyscall]",
 MemoryRegionInfos{
-MemoryRegionInfo(make_range(0x7fc090021000, 0x7fc09400),
- MemoryRegionInfo::eNo, MemoryRegionInfo::eNo,
- MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
- ConstString(nullptr),
- MemoryRegionInfo::eDontKnow, 0,
- MemoryRegionInfo::eDontKnow),
+MemoryRegionInfo(
+make_range(0x7fc090021000, 0x7fc09400),
+MemoryRegionInfo::eNo, MemoryRegionInfo::eNo,
+MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
+ConstString(nullptr), MemoryRegionInfo::eDontKnow, 0,
+MemoryRegionInfo::eDontKnow, MemoryRegionInfo::eDontKnow),
 MemoryRegionInfo(
 make_range(0xff60, 0xff601000),
 MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
 MemoryRegionInfo::eYes, MemoryRegionInfo::eYes,
 ConstString("[vsyscall]"), MemoryRegionInfo::eDontKnow, 0,
-MemoryRegionInfo::eDontKnow),
+MemoryRegionInfo::eDontKnow, MemoryRegionInfo::eDontKnow),
 },
 "")));
 
@@ -144,12 +145,12 @@ INSTANTIATE_TEST_SUITE_P(
 "- rw-p  00:00 0 [foo]\n"
 "0/0 rw-p  00:00 0",
 MemoryRegionInfos{
-MemoryRegionInfo(make_range(0x, 0x),
- MemoryRegionInfo::eYes, 
MemoryRegionInfo::eYes,
- MemoryRegionInfo::eNo, MemoryRegionInfo::eYes,
- ConstString("[foo]"),
- MemoryRegionInfo::eDontKnow, 0,
- MemoryRegionInfo::eDontKnow),
+MemoryRegionInfo(
+make_range(0x, 0x), MemoryRegionInfo::eYes,
+MemoryRegionInfo::eYes, MemoryRegionInfo::eNo,
+MemoryRegionInfo::eYes, ConstString("[foo]"),
+MemoryRegionInfo::eDontKnow, 0, 
MemoryRegionInfo::eDontKnow,
+MemoryRegionInfo::eDontKnow),
 },
 "malformed 

[Lldb-commits] [PATCH] D107931: [lldb] [gdb-remote] Implement vRun packet [WIP]

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

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

Implement the simpler vRun packet and prefer it over the A packet.
Unlike the latter, it tranmits command-line arguments without redundant
indices and lengths.

**TODO: add tests**


https://reviews.llvm.org/D107931

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Utility/StringExtractorGDBRemote.cpp

Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -363,6 +363,8 @@
 return eServerPacketType_vCont;
   if (PACKET_MATCHES("vCont?"))
 return eServerPacketType_vCont_actions;
+  if (PACKET_STARTS_WITH("vRun;"))
+return eServerPacketType_vRun;
 }
 break;
   case '_':
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -182,6 +182,9 @@
   RegisterMemberFunctionHandler(
   StringExtractorGDBRemote::eServerPacketType_vCont_actions,
   ::Handle_vCont_actions);
+  RegisterMemberFunctionHandler(
+  StringExtractorGDBRemote::eServerPacketType_vRun,
+  ::Handle_vRun);
   RegisterMemberFunctionHandler(
   StringExtractorGDBRemote::eServerPacketType_x,
   ::Handle_memory_read);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
@@ -39,6 +39,8 @@
 
   PacketResult Handle_A(StringExtractorGDBRemote );
 
+  PacketResult Handle_vRun(StringExtractorGDBRemote );
+
   PacketResult Handle_qHostInfo(StringExtractorGDBRemote );
 
   PacketResult Handle_qProcessInfoPID(StringExtractorGDBRemote );
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -1077,6 +1077,38 @@
   return SendErrorResponse(8);
 }
 
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerCommon::Handle_vRun(
+StringExtractorGDBRemote ) {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
+
+  llvm::StringRef s = packet.GetStringRef();
+  if (!s.consume_front("vRun;"))
+return SendErrorResponse(8);
+
+  llvm::SmallVector argv;
+  s.split(argv, ';');
+
+  for (llvm::StringRef hex_arg : argv) {
+StringExtractor arg_ext{hex_arg};
+std::string arg;
+arg_ext.GetHexByteString(arg);
+m_process_launch_info.GetArguments().AppendArgument(arg);
+LLDB_LOGF(log, "LLGSPacketHandler::%s added arg: \"%s\"", __FUNCTION__,
+  arg.c_str());
+  }
+
+  if (!argv.empty()) {
+m_process_launch_info.GetExecutableFile().SetFile(
+m_process_launch_info.GetArguments()[0].ref(), FileSpec::Style::native);
+m_process_launch_error = LaunchProcess();
+if (m_process_launch_error.Success())
+  return SendOKResponse();
+LLDB_LOG(log, "failed to launch exe: {0}", m_process_launch_error);
+  }
+  return SendErrorResponse(8);
+}
+
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_qEcho(
 StringExtractorGDBRemote ) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -598,7 +598,8 @@
   m_supports_qSymbol : 1, m_qSymbol_requests_done : 1,
   m_supports_qModuleInfo : 1, m_supports_jThreadsInfo : 1,
   m_supports_jModulesInfo : 1, m_supports_vFileSize : 1,
-  m_supports_vFileMode : 1, m_supports_vFileExists : 1;
+  m_supports_vFileMode : 1, m_supports_vFileExists : 1,
+  m_supports_vRun : 1;
 
   /// Current gdb remote protocol process identifier for all other operations
   lldb::pid_t m_curr_pid = LLDB_INVALID_PROCESS_ID;
Index: 

[Lldb-commits] [lldb] 767496d - [lldb] Skip TestStepOverWatchpoint on AS

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-08-11T13:43:12-07:00
New Revision: 767496d19cb9a1fbba57ff08095faa161998ee36

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

LOG: [lldb] Skip TestStepOverWatchpoint on AS

Include macosx in the list of operating systems for which this is broken
on arm64.

rdar://34027183

Added: 


Modified: 

lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py

Removed: 




diff  --git 
a/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py
 
b/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py
index c602dafb2bebd..d5240d5db68ad 100644
--- 
a/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py
+++ 
b/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py
@@ -21,7 +21,10 @@ class TestStepOverWatchpoint(TestBase):
 bugnumber="llvm.org/pr26031")
 # Read-write watchpoints not supported on SystemZ
 @expectedFailureAll(archs=['s390x'])
-@expectedFailureAll(oslist=["ios", "watchos", "tvos", "bridgeos"], 
bugnumber="")  # watchpoint tests aren't working on 
arm64
+@expectedFailureAll(
+oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
+archs=['aarch64', 'arm'],
+bugnumber="")
 @add_test_categories(["basic_process"])
 def test(self):
 """Test stepping over watchpoints."""



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

[Lldb-commits] [PATCH] D107625: Add a stack-memory-only style for Darwin process save-core

[Jason Molenda via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c31efeed600: Add the ability to process save-core 
stack-memory-only corefiles (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107625

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Target/MemoryRegionInfo.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
  lldb/test/API/macosx/stack-corefile/Makefile
  lldb/test/API/macosx/stack-corefile/TestStackCorefile.py
  lldb/test/API/macosx/stack-corefile/main.c
  lldb/tools/debugserver/source/DNBDefs.h
  lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
  lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp
  lldb/tools/debugserver/source/MacOSX/MachVMRegion.h
  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
@@ -4310,6 +4310,14 @@
   }
 }
 ostrm << ";";
+if (!region_info.vm_types.empty()) {
+  for (size_t i = 0; i < region_info.vm_types.size(); i++) {
+if (i)
+  ostrm << ",";
+ostrm << region_info.vm_types[i];
+  }
+  ostrm << ";";
+}
   }
   return SendPacket(ostrm.str());
 }
Index: lldb/tools/debugserver/source/MacOSX/MachVMRegion.h
===
--- lldb/tools/debugserver/source/MacOSX/MachVMRegion.h
+++ lldb/tools/debugserver/source/MacOSX/MachVMRegion.h
@@ -40,6 +40,7 @@
   vm_prot_t prot);
   bool RestoreProtections();
   bool GetRegionForAddress(nub_addr_t addr);
+  std::vector GetMemoryTypes() const;
 
   uint32_t GetDNBPermissions() const;
 
Index: lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp
===
--- lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp
+++ lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp
@@ -182,3 +182,43 @@
 dnb_permissions |= eMemoryPermissionsExecutable;
   return dnb_permissions;
 }
+
+std::vector MachVMRegion::GetMemoryTypes() const {
+  std::vector types;
+  if (m_data.user_tag == VM_MEMORY_STACK) {
+if (m_data.protection == VM_PROT_NONE) {
+  types.push_back("stack-guard");
+} else {
+  types.push_back("stack");
+}
+  }
+  if (m_data.user_tag == VM_MEMORY_MALLOC) {
+if (m_data.protection == VM_PROT_NONE)
+  types.push_back("malloc-guard");
+else if (m_data.share_mode == SM_EMPTY)
+  types.push_back("malloc-reserved");
+else
+  types.push_back("malloc-metadata");
+  }
+  if (m_data.user_tag == VM_MEMORY_MALLOC_NANO ||
+  m_data.user_tag == VM_MEMORY_MALLOC_TINY ||
+  m_data.user_tag == VM_MEMORY_MALLOC_SMALL ||
+  m_data.user_tag == VM_MEMORY_MALLOC_LARGE ||
+  m_data.user_tag == VM_MEMORY_MALLOC_LARGE_REUSED ||
+  m_data.user_tag == VM_MEMORY_MALLOC_LARGE_REUSABLE ||
+  m_data.user_tag == VM_MEMORY_MALLOC_HUGE ||
+  m_data.user_tag == VM_MEMORY_REALLOC ||
+  m_data.user_tag == VM_MEMORY_SBRK) {
+types.push_back("heap");
+if (m_data.user_tag == VM_MEMORY_MALLOC_TINY) {
+  types.push_back("malloc-tiny");
+}
+if (m_data.user_tag == VM_MEMORY_MALLOC_LARGE) {
+  types.push_back("malloc-large");
+}
+if (m_data.user_tag == VM_MEMORY_MALLOC_SMALL) {
+  types.push_back("malloc-small");
+}
+  }
+  return types;
+}
Index: lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
===
--- lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
+++ lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
@@ -125,6 +125,7 @@
 region_info->permissions = vmRegion.GetDNBPermissions();
 region_info->dirty_pages =
 get_dirty_pages(task, vmRegion.StartAddress(), vmRegion.GetByteSize());
+region_info->vm_types = vmRegion.GetMemoryTypes();
   } else {
 region_info->addr = address;
 region_info->size = 0;
Index: lldb/tools/debugserver/source/DNBDefs.h
===
--- lldb/tools/debugserver/source/DNBDefs.h
+++ lldb/tools/debugserver/source/DNBDefs.h
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -318,11 +319,13 @@
 
 struct DNBRegionInfo {
 public:
-  DNBRegionInfo() : addr(0), size(0), permissions(0), dirty_pages() {}
+  DNBRegionInfo()
+  : 

[Lldb-commits] [lldb] 8c31efe - Add the ability to process save-core stack-memory-only corefiles

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2021-08-11T13:37:31-07:00
New Revision: 8c31efeed6007bade1e4ddda2ceccab311661cbc

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

LOG: Add the ability to process save-core stack-memory-only corefiles

Add a field to the qMemoryRegionInfo packet where the remote stub
can describe the type of memory -- heap, stack.  Keep track of
memory regions that are stack memory in lldb.  Add a new "--style
stack" to process save-core to request that only stack memory be
included in the corefile.

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

Added: 
lldb/test/API/macosx/stack-corefile/Makefile
lldb/test/API/macosx/stack-corefile/TestStackCorefile.py
lldb/test/API/macosx/stack-corefile/main.c

Modified: 
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Target/MemoryRegionInfo.h
lldb/include/lldb/lldb-enumerations.h
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
lldb/tools/debugserver/source/DNBDefs.h
lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp
lldb/tools/debugserver/source/MacOSX/MachVMRegion.h
lldb/tools/debugserver/source/RNBRemote.cpp

Removed: 




diff  --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index 3eb3dc51c0280..17588021e3139 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -1206,6 +1206,9 @@ tuples to return are:
   // is "mt" for AArch64 memory tagging. lldb will
   // ignore any other flags in this field.
 
+type:[][,]; // memory types that apply to this region, e.g.
+ // "stack" for stack memory.
+
 error:; // where  is
  // a hex encoded string value that
  // contains an error string

diff  --git a/lldb/include/lldb/Target/MemoryRegionInfo.h 
b/lldb/include/lldb/Target/MemoryRegionInfo.h
index c43f27e0c366b..fc5fcff5159ee 100644
--- a/lldb/include/lldb/Target/MemoryRegionInfo.h
+++ b/lldb/include/lldb/Target/MemoryRegionInfo.h
@@ -28,10 +28,10 @@ class MemoryRegionInfo {
   MemoryRegionInfo(RangeType range, OptionalBool read, OptionalBool write,
OptionalBool execute, OptionalBool mapped, ConstString name,
OptionalBool flash, lldb::offset_t blocksize,
-   OptionalBool memory_tagged)
+   OptionalBool memory_tagged, OptionalBool stack_memory)
   : m_range(range), m_read(read), m_write(write), m_execute(execute),
 m_mapped(mapped), m_name(name), m_flash(flash), m_blocksize(blocksize),
-m_memory_tagged(memory_tagged) {}
+m_memory_tagged(memory_tagged), m_is_stack_memory(stack_memory) {}
 
   RangeType () { return m_range; }
 
@@ -98,7 +98,8 @@ class MemoryRegionInfo {
m_mapped == rhs.m_mapped && m_name == rhs.m_name &&
m_flash == rhs.m_flash && m_blocksize == rhs.m_blocksize &&
m_memory_tagged == rhs.m_memory_tagged &&
-   m_pagesize == rhs.m_pagesize;
+   m_pagesize == rhs.m_pagesize &&
+   m_is_stack_memory == rhs.m_is_stack_memory;
   }
 
   bool operator!=(const MemoryRegionInfo ) const { return !(*this == rhs); 
}
@@ -116,6 +117,10 @@ class MemoryRegionInfo {
 return m_dirty_pages;
   }
 
+  OptionalBool IsStackMemory() const { return m_is_stack_memory; }
+
+  void SetIsStackMemory(OptionalBool val) { m_is_stack_memory = val; }
+
   void SetPageSize(int pagesize) { m_pagesize = pagesize; }
 
   void SetDirtyPageList(std::vector pagelist) {
@@ -134,6 +139,7 @@ class MemoryRegionInfo {
   OptionalBool m_flash = eDontKnow;
   lldb::offset_t m_blocksize = 0;
   OptionalBool m_memory_tagged = eDontKnow;
+  OptionalBool m_is_stack_memory = eDontKnow;
   int m_pagesize = 0;
   llvm::Optional> m_dirty_pages;
 };

diff  --git a/lldb/include/lldb/lldb-enumerations.h 
b/lldb/include/lldb/lldb-enumerations.h
index 81f6be3eec7d8..f5cabc02bd84e 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -1137,6 +1137,7 @@ enum SaveCoreStyle {
   eSaveCoreUnspecified = 0,
   eSaveCoreFull = 1,
   eSaveCoreDirtyOnly = 2,
+  eSaveCoreStackOnly = 3,
 };
 
 } // namespace lldb

diff  --git a/lldb/source/Commands/CommandObjectProcess.cpp 
b/lldb/source/Commands/CommandObjectProcess.cpp
index 1a8ed021b9f0e..c65b661f9cfc9 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -1166,7 +1166,9 @@ class 

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D107869#2940016 <https://reviews.llvm.org/D107869#2940016>, @OmarEmaraDev 
wrote:

> @jingham I wasn't arguing that we should remove those environment variables, 
> on the contrary. Greg was suggesting that we populate the environment field 
> with the target environment instead of implicitly adding them to the 
> environment of the launch info. The problem with that is that there is a 
> large number of environments that gets added (15 in my case) as shown in the 
> following screenshot. What I was saying in my comment above is that I don't 
> think we should show those to the user, we should transparently add them 
> instead and let the user add additional variables as needed. We can then look 
> into adding other settings to exclude them if necessary.
>
> F18486746: 20210811-210121.png <https://reviews.llvm.org/F18486746>

Thanks for the clarification.

I still think people will want to see the actual environment that is being 
provided to the inferior.  They don't want to have to do our merging algorithm 
in their heads.

For instance, if lldb had an environment variable that wasn't appropriate for 
the inferior, I would want to see that in the Environment View, so I could turn 
it off.  If that variable is passed silently to the inferior, and not shown 
anywhere in the launch UI, figuring out that I needed to not inherit that one 
would be more difficult.

However, if you feel like the really useful affordance here is for "User 
Provided Environment Variables", that's also a valid choice, but then you 
should name it appropriately, so as not to cause confusion.  If there's a 
window listing in the launch parameters panel called "Environment Variables" it 
should be the environment variables the process will launch with, since that's 
the plain meaning of the title.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107869

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

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev updated this revision to Diff 365825.
OmarEmaraDev added a comment.

- Address review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107869

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -28,6 +28,7 @@
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/ValueObjectUpdater.h"
 #include "lldb/Host/File.h"
+#include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/Predicate.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StreamString.h"
@@ -1260,6 +1261,14 @@
 
   const std::string () { return m_content; }
 
+  void SetText(const char *text) {
+if (text == nullptr) {
+  m_content.clear();
+  return;
+}
+m_content = text;
+  }
+
 protected:
   std::string m_label;
   bool m_required;
@@ -1619,6 +1628,33 @@
   }
 };
 
+class LazyBooleanFieldDelegate : public ChoicesFieldDelegate {
+public:
+  LazyBooleanFieldDelegate(const char *label, const char *calculate_label)
+  : ChoicesFieldDelegate(label, 3, GetPossibleOptions(calculate_label)) {}
+
+  static constexpr const char *kNo = "No";
+  static constexpr const char *kYes = "Yes";
+
+  std::vector GetPossibleOptions(const char *calculate_label) {
+std::vector options;
+options.push_back(calculate_label);
+options.push_back(kYes);
+options.push_back(kNo);
+return options;
+  }
+
+  LazyBool GetLazyBoolean() {
+std::string choice = GetChoiceContent();
+if (choice == kNo)
+  return eLazyBoolNo;
+else if (choice == kYes)
+  return eLazyBoolYes;
+else
+  return eLazyBoolCalculate;
+  }
+};
+
 template  class ListFieldDelegate : public FieldDelegate {
 public:
   ListFieldDelegate(const char *label, T default_field)
@@ -1909,6 +1945,29 @@
   SelectionType m_selection_type;
 };
 
+class ArgumentsFieldDelegate : public ListFieldDelegate {
+public:
+  ArgumentsFieldDelegate()
+  : ListFieldDelegate("Arguments",
+  TextFieldDelegate("Argument", "", false)) {}
+
+  Args GetArguments() {
+Args arguments;
+for (int i = 0; i < GetNumberOfFields(); i++) {
+  arguments.AppendArgument(GetField(i).GetText());
+}
+return arguments;
+  }
+
+  void AddArguments(const Args ) {
+for (size_t i = 0; i < arguments.GetArgumentCount(); i++) {
+  AddNewField();
+  TextFieldDelegate  = GetField(GetNumberOfFields() - 1);
+  field.SetText(arguments.GetArgumentAtIndex(i));
+}
+  }
+};
+
 template 
 class MappingFieldDelegate : public FieldDelegate {
 public:
@@ -2069,6 +2128,10 @@
   const std::string () { return GetKeyField().GetName(); }
 
   const std::string () { return GetValueField().GetText(); }
+
+  void SetName(const char *name) { return GetKeyField().SetText(name); }
+
+  void SetValue(const char *value) { return GetValueField().SetText(value); }
 };
 
 class EnvironmentVariableListFieldDelegate
@@ -2077,6 +2140,25 @@
   EnvironmentVariableListFieldDelegate()
   : ListFieldDelegate("Environment Variables",
   EnvironmentVariableFieldDelegate()) {}
+
+  Environment GetEnvironment() {
+Environment environment;
+for (int i = 0; i < GetNumberOfFields(); i++) {
+  environment.insert(
+  std::make_pair(GetField(i).GetName(), GetField(i).GetValue()));
+}
+return environment;
+  }
+
+  void AddEnvironmentVariables(const Environment ) {
+for (auto  : environment) {
+  AddNewField();
+  EnvironmentVariableFieldDelegate  =
+  GetField(GetNumberOfFields() - 1);
+  field.SetName(variable.getKey().str().c_str());
+  field.SetValue(variable.getValue().c_str());
+}
+  }
 };
 
 class FormAction {
@@ -2201,6 +2283,14 @@
 return delegate;
   }
 
+  LazyBooleanFieldDelegate *AddLazyBooleanField(const char *label,
+const char *calculate_label) {
+LazyBooleanFieldDelegate *delegate =
+new LazyBooleanFieldDelegate(label, calculate_label);
+m_fields.push_back(FieldDelegateUP(delegate));
+return delegate;
+  }
+
   ChoicesFieldDelegate *AddChoicesField(const char *label, int height,
 std::vector choices) {
 ChoicesFieldDelegate *delegate =
@@ -2230,6 +2320,12 @@
 return delegate;
   }
 
+  ArgumentsFieldDelegate *AddArgumentsField() {
+ArgumentsFieldDelegate *delegate = new ArgumentsFieldDelegate();
+m_fields.push_back(FieldDelegateUP(delegate));
+return delegate;
+  }
+
   template 
   MappingFieldDelegate *AddMappingField(K key_field, V value_field) {
 MappingFieldDelegate *delegate =
@@ -3031,6 +3127,359 @@
   ChoicesFieldDelegate *m_load_dependent_files_field;
 };
 
+class ProcessLaunchFormDelegate : public FormDelegate {

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev added a comment.

@jingham I wasn't arguing that we should remove those environment variables, on 
the contrary. Greg was suggesting that we populate the environment field with 
the target environment instead of implicitly adding them to the environment of 
the launch info. The problem with that is that there is a large number of 
environments that gets added (15 in my case) as shown in the following 
screenshot. What I was saying in my comment above is that I don't think we 
should show those to the user, we should transparently add them instead and let 
the user add additional variables as needed. We can then look into adding other 
settings to exclude them if necessary.

F18486746: 20210811-210121.png <https://reviews.llvm.org/F18486746>


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107869

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

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D107869#2939922 , @OmarEmaraDev 
wrote:

> One thing to note as well is that the target environment seem to include many 
> environment variables like PATH and HOME, I don't think those should be 
> included.

If I am debugging a program on my local machine then I do want the program to 
start up with reasonable values for PATH and HOME, DISPLAY and suchlike or it 
won't run correctly.  Since people don't tend to have a lot of "debugger 
specific environment" variables set, it's just a fairly vanilla command-line 
tool in that regard, the debugger's environment is a fine thing to use to prime 
the inferior.  Note, if there are certain environment variables that you don't 
want to pass from the debugger to the inferior, use the setting 
target.unset-env-vars.

Of course, if you are debugging remotely to a device or to another machine, you 
probably don't want to share many (or any) env-vars with the inferior.  In that 
use-case, you set target.inherit-env to 0 and fill in all the necessary ones 
with target.env-vars.

But I think the most common use-case is local debugging, and lldb is usually 
run with a fairly reasonable environment, so having target.inherit-env be true 
by default is the right choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107869

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

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev added a comment.

One thing to note as well is that the target environment seem to include many 
environment variables like PATH and HOME, I don't think those should be 
included.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107869

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

[Lldb-commits] [PATCH] D106743: [lldb][NFC] Define DWARFDIE::children out-of-line instead of using template magic

[Raphael Isemann 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 rGb2c262cfb12f: [lldb][NFC] Define DWARFDIE::children 
out-of-line instead of using template… (authored by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106743

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -90,14 +90,9 @@
 int _line, int _column, int _file,
 int _line, int _column,
 lldb_private::DWARFExpression *frame_base) const;
+
   /// The range of all the children of this DIE.
-  ///
-  /// This is a template just because child_iterator is not completely defined
-  /// at this point.
-  template 
-  llvm::iterator_range children() const {
-return llvm::make_range(T(*this), T());
-  }
+  llvm::iterator_range children() const;
 };
 
 class DWARFDIE::child_iterator
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -448,3 +448,7 @@
   } else
 return false;
 }
+
+llvm::iterator_range DWARFDIE::children() const {
+  return llvm::make_range(child_iterator(*this), child_iterator());
+}


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -90,14 +90,9 @@
 int _line, int _column, int _file,
 int _line, int _column,
 lldb_private::DWARFExpression *frame_base) const;
+
   /// The range of all the children of this DIE.
-  ///
-  /// This is a template just because child_iterator is not completely defined
-  /// at this point.
-  template 
-  llvm::iterator_range children() const {
-return llvm::make_range(T(*this), T());
-  }
+  llvm::iterator_range children() const;
 };
 
 class DWARFDIE::child_iterator
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -448,3 +448,7 @@
   } else
 return false;
 }
+
+llvm::iterator_range DWARFDIE::children() const {
+  return llvm::make_range(child_iterator(*this), child_iterator());
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] b2c262c - [lldb][NFC] Define DWARFDIE::children out-of-line instead of using template magic

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-08-11T19:19:41+02:00
New Revision: b2c262cfb12f4fb43177759426b1c6128d4236e4

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

LOG: [lldb][NFC] Define DWARFDIE::children out-of-line instead of using 
template magic

As pointed out by David in D103172 (thanks!)

Reviewed By: dblaikie

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index dda691eecaccc..529007e31b9e5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -448,3 +448,7 @@ bool DWARFDIE::GetDIENamesAndRanges(
   } else
 return false;
 }
+
+llvm::iterator_range DWARFDIE::children() const {
+  return llvm::make_range(child_iterator(*this), child_iterator());
+}

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
index 56154055c44dc..5ee44a7632049 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -90,14 +90,9 @@ class DWARFDIE : public DWARFBaseDIE {
 int _line, int _column, int _file,
 int _line, int _column,
 lldb_private::DWARFExpression *frame_base) const;
+
   /// The range of all the children of this DIE.
-  ///
-  /// This is a template just because child_iterator is not completely defined
-  /// at this point.
-  template 
-  llvm::iterator_range children() const {
-return llvm::make_range(T(*this), T());
-  }
+  llvm::iterator_range children() const;
 };
 
 class DWARFDIE::child_iterator



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

[Lldb-commits] [lldb] be556d5 - [lldb/Commands] Fix heap-use-after-free error in CommandObjectProcess

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2021-08-11T17:03:20+01:00
New Revision: be556d5131d56f285e55b0548f3b953d55d055c4

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

LOG: [lldb/Commands] Fix heap-use-after-free error in CommandObjectProcess

This patch should fix the use-after-free error that was brought up by
the LLDB ASAN Green Dragon bot.

This is caused because the `StringRef` object was acquired too early
before being use and by the underlying memory was modified which caused
it to point to null memory.

Fetching back the string reference close to its usage location should
fix the issue.

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/source/Commands/CommandObjectProcess.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectProcess.cpp 
b/lldb/source/Commands/CommandObjectProcess.cpp
index 7aaba37315000..1a8ed021b9f0e 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -170,8 +170,6 @@ class CommandObjectProcessLaunch : public 
CommandObjectProcessLaunchOrAttach {
 if (!StopProcessIfNecessary(m_exe_ctx.GetProcessPtr(), state, result))
   return false;
 
-llvm::StringRef target_settings_argv0 = target->GetArg0();
-
 // Determine whether we will disable ASLR or leave it in the default state
 // (i.e. enabled if the platform supports it). First check if the process
 // launch options explicitly turn on/off
@@ -216,6 +214,8 @@ class CommandObjectProcessLaunch : public 
CommandObjectProcessLaunchOrAttach {
 m_options.launch_info.GetEnvironment().insert(target_env.begin(),
   target_env.end());
 
+llvm::StringRef target_settings_argv0 = target->GetArg0();
+
 if (!target_settings_argv0.empty()) {
   m_options.launch_info.GetArguments().AppendArgument(
   target_settings_argv0);



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

[Lldb-commits] [PATCH] D106466: 3/3: [llvm+lldb] Fix#2 of DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

[Igor Kudrin via Phabricator via lldb-commits]

ikudrin added a comment.

In D106466#2926185 , @jankratochvil 
wrote:

> In D106466#2922549 , @ikudrin wrote:
>
>> As far as I understand it, you need a specially constructed 
>> `llvm::DWARFDebugRnglistTable` object so that 
>> `DWARFUnit::FindRnglistFromOffset()` can call its `findList()` method and 
>> get a list of records located at a specific offset.
>
> Yes.
>
>> It looks like a newly created `llvm::DWARFDebugRnglistTable` object has its 
>> `Header.Length` set to `0`,
>
> But that is wrong `Length`, the correct one is to cover the whole section 
> which I set by:
>
>   HeaderData.Length = Data.size() - dwarf::getUnitLengthFieldByteSize(Format);

This value is not incorrect, but a special one, with custom handling. 
`DWARFUnit::findRnglistFromOffset(uint64_t Offset)` fetches the list without 
creating a fake header.

>> so it already works as required for the usage, and there is no need to fill 
>> its fields with artificial values that do not represent real content of the 
>> section. Am I right?
>
> One needs to set at least `AddrSize` and `OffsetEntryCount` as callers do use 
> it.

Could you please point me to that usage? I am not that familiar with the LLDB 
code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106466

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

[Lldb-commits] [PATCH] D107470: 2/3: [llvm+lldb] Remove dead-code in DWARFListTableHeader::extract modifying DWARFDataExtractor

[Igor Kudrin via Phabricator via lldb-commits]

ikudrin added inline comments.



Comment at: llvm/unittests/DebugInfo/DWARF/DWARFListTableTest.cpp:128
+  EXPECT_EQ(Table.getAddrSize(), 8U);
+  Extractor.setAddressSize(Table.getAddrSize());
+  Expected List = Table.findList(Extractor, Offset);

This looks odd. `DWARFListTableBase::findList()` should not require the setting 
to be done in the calling code if it can apply it itself. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107470

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

[Lldb-commits] [PATCH] D107079: [lldb] Add an option for emitting LLDB logs as JSON

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D107079#2914111 , @aprantl wrote:

> More a comment than anything else: One thing I always wanted to explore was 
> to implement LLDB's logging on Darwin on top of os_log 
> (https://developer.apple.com/documentation/os/logging) which is also 
> structured and also faster since it moves the the formatting out of process. 
> This patch is great because it also works for non-Darwin platforms, and I 
> guess there isn't anything preventing us from moving to os_log even if we 
> take patch.

Just logging messages to os_log shouldn't be a problem and I can do that as a 
follow-up. I don't think we can really use the optimization part where it 
stores the format string + args as that would require that everything is always 
using the old printf-style logging (which `LLDB_LOG` and `LLDB_LOG_ERROR` 
replace with a type safe API that uses different format strings).


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

https://reviews.llvm.org/D107079

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

[Lldb-commits] [PATCH] D107079: [lldb] Add an option for emitting LLDB logs as JSON

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D107079#2918185 , @jasonmolenda 
wrote:

> Sorry for not commenting on this on earlier, I wanted to think about it a bit.
>
> I think the usefulness of this can depend on the type of logging.  l often 
> turn on gdb-remote packet logging, or thread step logging, in an interactive 
> debug session and watch the log messages streaming as I do lldb commands.  I 
> think interposing a JSON formatter in this is less straightforward - not that 
> reading JSON is that difficult, but it's just adding layering between me and 
> the log messages.  WIth this patch the logging will be like {msg="..."} and 
> fine, but we'll add more fields to this over time and it will become less 
> easy to read.
>
> Maybe this can be an *option* to logging like verbose logging is.  I don't 
> have any problem with the default mode being JSON and people who want rawness 
> can add an option to log enable to not do that.
>
> I can't get too high and mighty with my philosophy of disintermediation -- 
> gdb-remote packets are often actually using binary data, and even compressed, 
> and lldb reformats all of that before logging so my terminal doesn't get 
> borked by escape/high-bit characters being dumped on to it. But I read the 
> gdb-remote packet logs every day and wrapping it in JSON doesn't make 
> anything better for this use case, I don't think.  This may be more a Jason 
> opinion than a gdb-remote packet reader opinion!

Yeah, I actually wanted to write finish my comment above that started as "Do we 
want to keep the old plain text format too? To be honest, I don't really see 
much" with "point in that but I don't mind keeping the old format around if 
someone thinks its useful." Somehow that part of my sentence got deleted. 
Anyway, the plain text format is still around and still the default so I think 
this concern is addressed.

In D107079#2914189 , @jingham wrote:

> When you are using the logging for a reliably reproducible problem (some 
> parsing issue, for instance) then there's no harm to the general "turn on all 
> logging and we'll sort it out on our end".  But for any issues that are 
> timing sensitive the problem is that too much logging changes the timing and 
> often causes bugs to disappear.  This patch will add volume to log emission 
> and slow its emission down, so it goes a bit in the wrong direction.  I don't 
> think that's a reason NOT to do this, hopefully the overhead is small.  But 
> it means we can't assume "turning on all logs" is a reasonable default 
> behavior for logging.  If anything, we need to go in the other direction and 
> add a more fine-grained verbosity so we can ask for "a bit of log channel A 
> and a bit of B and more of C" or whatever.

I think this is also addressed by having the plain text format around in its 
current form.

> I'm not entirely sure what you intend when you say:
>
>> Every log message is now a serialized JSON object (that is now also 
>> guaranteed to occupy exactly one line) that is just appended to the log file.
>
> Log messages are going to have new-lines in them because it's quite 
> convenient to use the various object Description or Dump methods to log 
> objects and those are generally laid out to be human readable (and are used 
> for that purpose in other contexts.)  By "single line" it seems you mean "all 
> log messages with new lines will have the newline converted to "\n" as a 
> character string before being inserted into the JSON.  This seems more an 
> implementation detail, I'm not sure why you call it out.  In any case, 
> preserving the multi-line structure of log content is important for 
> eventually being able to read them, so as long as the encoders & printers do 
> the right thing, it doesn't really matter how they are encoded.

This is just about the fact that every JSON log message is guaranteed to be one 
line (even if the message contains newlines which will just be escaped). You 
can still log multiline messages as before  The `1 object = 1 line` property is 
just relevant for my comment below: `What about logs that are printed to stdin 
and we want to parse? I think you can easily make a tool that filters out the 
non-JSON output. Logs are guaranteed to have every object on its own line, so 
you can just go line-by-line and filter them based on that.`. It's technically 
also possible now with both formats around to have a plain text channel and a 
JSON channel print to the same file, so because of this `1 object = 1 line` 
property that is something that log parser could handle if we wanted to. Not 
that we should ever actually tell someone to log both formats into one file, 
but at least the final output would be salvageable this way.

> Conversely, however, it is NOT the case that each LLDB_LOGF call is a single 
> Log message.  For instance, at the beginning and end of each round of 
> "ShouldStop" negotiation, I 

[Lldb-commits] [PATCH] D107456: [lldb] Support .debug_rnglists.dwo sections in dwp file

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:519
+contribution->Length);
+return DWARFDataExtractor();
+  }

Here could be an error message.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:537
+if (const llvm::DWARFUnitIndex::Entry *entry = m_header.GetIndexEntry()) {
+  const auto *contribution = 
entry->getContribution(llvm::DW_SECT_RNGLISTS);
+  if (!contribution) {

I have found this `getContribution` adjustment duplication, are there some 
reasons it is not unified?  https://www.jankratochvil.net/t/D107456-unify.patch
The issue is the code then handles two different `.debug_rnglists` 
`DWARFDataExtractor`s with different offsets.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:556
+  " (ranges list base: 0x%" PRIx64 "): %s",
+  offset, m_ranges_base, toString(table_or_error.takeError()).c_str());
   }

One such reason can be missing DWP absolute offset for the error report. That 
could be returned from `GetRnglistData()`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107456

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

[Lldb-commits] [PATCH] D107079: [lldb] Add an option for emitting LLDB logs as JSON

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 365737.
teemperor retitled this revision from "[lldb] Change the log format to JSON 
instead of plain text" to "[lldb] Add an option for emitting LLDB logs as JSON".
teemperor edited the summary of this revision.
teemperor added a comment.
Herald added subscribers: dang, jfb, arphaman.

- Made the logging format optional so both JSON and plain text can be used 
(with plain text the default). It's not a channel option like everything else.
- Added the log format as an option to `log enable` (`o`, `f` and `F` are 
taken, so I just picked `O` as the short option).
- Added documentation for logging and the log format to the website.
- Added some tests for the possibility of having mixed plain and JSON logging 
in one file.
- Removed the API test changes that tried to do JSON parsing instead of parsing 
the plain text (this might still be a good idea in the future in a separate 
patch).


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

https://reviews.llvm.org/D107079

Files:
  lldb/docs/design/logging.rst
  lldb/docs/index.rst
  lldb/include/lldb/Interpreter/CommandCompletions.h
  lldb/include/lldb/Utility/Log.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/Options.td
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Utility/Log.cpp
  lldb/test/API/commands/log/invalid-args/TestInvalidArgsLog.py
  lldb/test/API/functionalities/completion/TestCompletion.py
  lldb/test/API/functionalities/logging/Makefile
  lldb/test/API/functionalities/logging/TestLogging.py
  lldb/test/API/functionalities/logging/main.cpp
  lldb/unittests/Utility/LogTest.cpp

Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -11,6 +11,7 @@
 
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Threading.h"
 #include 
@@ -82,6 +83,16 @@
   llvm::StringRef takeOutput();
   llvm::StringRef logAndTakeOutput(llvm::StringRef Message);
 
+  llvm::json::Object logAndTakeLogObject(llvm::StringRef Message) {
+llvm::StringRef Out = logAndTakeOutput(Message).trim();
+EXPECT_TRUE(Out.consume_back(","));
+llvm::Expected parsed = llvm::json::parse(Out);
+EXPECT_TRUE(static_cast(parsed));
+llvm::json::Object *parsed_obj = parsed->getAsObject();
+EXPECT_NE(parsed_obj, nullptr);
+return *parsed_obj;
+  }
+
 public:
   void SetUp() override;
 };
@@ -92,6 +103,11 @@
 
   std::string error;
   ASSERT_TRUE(EnableChannel(m_stream_sp, 0, "chan", {}, error));
+  llvm::raw_string_ostream error_stream(error);
+  // Use JSON logging by default so the log messages can be parsed.
+  ASSERT_TRUE(
+  Log::SetLogChannelFormat("chan", Log::OutputFormat::JSON, error_stream));
+  EXPECT_EQ(error, "");
 
   m_log = test_channel.GetLogIfAll(FOO);
   ASSERT_NE(nullptr, m_log);
@@ -211,37 +227,66 @@
 
 TEST_F(LogChannelEnabledTest, log_options) {
   std::string Err;
-  EXPECT_EQ("Hello World\n", logAndTakeOutput("Hello World"));
+  EXPECT_EQ("{\"message\":\"Hello World\"},\n",
+logAndTakeOutput("Hello World"));
   EXPECT_TRUE(EnableChannel(getStream(), LLDB_LOG_OPTION_THREADSAFE, "chan", {},
 Err));
-  EXPECT_EQ("Hello World\n", logAndTakeOutput("Hello World"));
+  EXPECT_EQ("{\"message\":\"Hello World\"},\n",
+logAndTakeOutput("Hello World"));
 
   {
 EXPECT_TRUE(EnableChannel(getStream(), LLDB_LOG_OPTION_PREPEND_SEQUENCE,
   "chan", {}, Err));
-llvm::StringRef Msg = logAndTakeOutput("Hello World");
-int seq_no;
-EXPECT_EQ(1, sscanf(Msg.str().c_str(), "%d Hello World", _no));
+llvm::json::Object parsed_obj = logAndTakeLogObject("Hello World");
+// Check that any sequence-id was added.
+EXPECT_TRUE(parsed_obj.getInteger("sequence-id"));
   }
 
   {
 EXPECT_TRUE(EnableChannel(getStream(), LLDB_LOG_OPTION_PREPEND_FILE_FUNCTION,
   "chan", {}, Err));
-llvm::StringRef Msg = logAndTakeOutput("Hello World");
-char File[12];
-char Function[17];
-  
-sscanf(Msg.str().c_str(), "%[^:]:%s Hello World", File, Function);
-EXPECT_STRCASEEQ("LogTest.cpp", File);
-EXPECT_STREQ("logAndTakeOutput", Function);
+llvm::json::Object parsed_obj = logAndTakeLogObject("Hello World");
+// Check that function/file parameters were added.
+llvm::Optional function = parsed_obj.getString("function");
+llvm::Optional file = parsed_obj.getString("file");
+EXPECT_TRUE(function.hasValue());
+EXPECT_TRUE(file.hasValue());
+
+EXPECT_EQ(*function, "logAndTakeOutput");
+EXPECT_EQ(*file, "LogTest.cpp");
   }
 
-  EXPECT_TRUE(EnableChannel(

[Lldb-commits] [lldb] b136290 - [lldb][docs] Remove index entry to removed StructuredDataPlugins

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-08-11T13:47:04+02:00
New Revision: b136290bc46c6751ea59acfa97d7ac5ca044ded1

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

LOG: [lldb][docs] Remove index entry to removed StructuredDataPlugins

This page was removed in b2e25572d2a7b65a018580097b50910b3049ab65

Added: 


Modified: 
lldb/docs/index.rst

Removed: 




diff  --git a/lldb/docs/index.rst b/lldb/docs/index.rst
index ffac28a76454..1fdf674863e7 100644
--- a/lldb/docs/index.rst
+++ b/lldb/docs/index.rst
@@ -159,7 +159,6 @@ interesting areas to contribute to lldb.
 
design/overview
design/reproducers
-   design/structureddataplugins
design/sbapi
 
 .. toctree::



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

[Lldb-commits] [lldb] f6748b2 - [lldb][NFC] Fix small format error in TestCppVirtualFunctions

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-08-11T12:26:56+02:00
New Revision: f6748b24d4d731b3b60d97c20a1a29e66472ce66

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

LOG: [lldb][NFC] Fix small format error in TestCppVirtualFunctions

Added: 


Modified: 
lldb/test/API/lang/cpp/virtual-functions/TestCppVirtualFunctions.py

Removed: 




diff  --git 
a/lldb/test/API/lang/cpp/virtual-functions/TestCppVirtualFunctions.py 
b/lldb/test/API/lang/cpp/virtual-functions/TestCppVirtualFunctions.py
index 32c4d3513974a..4cbbc86fd1db0 100644
--- a/lldb/test/API/lang/cpp/virtual-functions/TestCppVirtualFunctions.py
+++ b/lldb/test/API/lang/cpp/virtual-functions/TestCppVirtualFunctions.py
@@ -9,7 +9,7 @@ class TestCase(TestBase):
 
 def common_setup(self):
 self.build()
-lldbutil.run_to_source_breakpoint(self,"// break here", 
lldb.SBFileSpec("main.cpp"))
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"))
 
 def test_call_on_base(self):
 self.common_setup()



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

[Lldb-commits] [lldb] 7377997 - [lldb] Rework 'lldb' substitution workaround in dwarf5-lazy-dwo.c

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-08-11T12:12:25+02:00
New Revision: 737799713451a59ec2c2a84ea2daeceaf32ff1d5

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

LOG: [lldb] Rework 'lldb' substitution workaround in dwarf5-lazy-dwo.c

This test is specifying the lldb log channel via `ll""db` which only really 
works
because the command parser ends up parsing that as `lldb`. Just putting the
channel name in quotes is enough to avoid the lldb command substitution and
doesn't rely on this weird parser behaviour.

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c 
b/lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c
index 7e5a44035df71..5c9f1fc60976e 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c
+++ b/lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c
@@ -8,7 +8,7 @@
 // RUN: %clang_host %s -fno-standalone-debug -g \
 // RUN:   -gdwarf-5 -gpubnames -gsplit-dwarf -c -o %t2.o -DTWO
 // RUN: %clang_host %t1.o %t2.o -o %t
-// RUN: %lldb %t -o "log enable ll""db object" -o "settings set 
stop-line-count-before 0" \
+// RUN: %lldb %t -o "log enable 'lldb' object" -o "settings set 
stop-line-count-before 0" \
 // RUN:   -o "b main" -o "run" -o "image lookup -n main -v" -b | FileCheck %s
 
 // CHECK-NOT: 2.dwo,



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

[Lldb-commits] [PATCH] D100206: [lldb] [llgs client] Support minimal fork/vfork handling

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

mgorny added a comment.

@JDevlieghere, any chance you could look at this, please?


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

https://reviews.llvm.org/D100206

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

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev added inline comments.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3237-3240
+} else {
+  action.Open(STDIN_FILENO, dev_null, true, false);
+  launch_info.AppendFileAction(action);
+}

clayborg wrote:
> We don't need to do anything if this isn't specified as by default the input 
> will be hooked up by the debugging to something valid. If the users wants to 
> redirect to /dev/null, they can just specify "/dev/null" (or the right path 
> for this on their system.
But since we are in GUI mode, what will the standard files be hooked into? It 
seems the only two options is to either redirect to a file or to /dev/null, 
hence the condition I have. Is this not the case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107869

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