[Lldb-commits] [PATCH] D125325: Pass plugin_name in SBProcess::SaveCore
PatriosTheGreat added a comment. Thanks Greg, Can you or someone please take this commit to main branch since I don't have a commit permission? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125325/new/ https://reviews.llvm.org/D125325 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 13e1cf8 - Reland "[lldb] Add --all option to "memory region""
Author: David Spickett
Date: 2022-05-19T13:16:36+01:00
New Revision: 13e1cf806567bc4987817e14a774198c3e3f2709
URL:
https://github.com/llvm/llvm-project/commit/13e1cf806567bc4987817e14a774198c3e3f2709
DIFF:
https://github.com/llvm/llvm-project/commit/13e1cf806567bc4987817e14a774198c3e3f2709.diff
LOG: Reland "[lldb] Add --all option to "memory region""
This reverts commit 3e928c4b9dfb01efd2cb968795e605760828e873.
This fixes an issue seen on Windows where we did not properly
get the section names of regions if they overlapped. Windows
has regions like:
[0x7fff928db000-0x7fff949a) ---
[0x7fff949a-0x7fff949a1000) r-- PECOFF header
[0x7fff949a-0x7fff94a3d000) r-x .hexpthk
[0x7fff949a-0x7fff94a85000) r-- .rdata
[0x7fff949a-0x7fff94a88000) rw- .data
[0x7fff949a-0x7fff94a94000) r-- .pdata
[0x7fff94a94000-0x7fff9525) ---
I assumed that you could just resolve the address and get the section
name using the start of the region but here you'd always get
"PECOFF header" because they all have the same start point.
The usual command repeating loop used the end address of the previous
region when requesting the next, or getting the section name.
So I've matched this in the --all scenario.
In the example above, somehow asking for the region at
0x7fff949a1000 would get you a region that starts at
0x7fff949a but has a different end point. Using the load
address you get (what I assume is) the correct section name.
Added:
Modified:
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Commands/Options.td
lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
llvm/docs/ReleaseNotes.rst
Removed:
diff --git a/lldb/source/Commands/CommandObjectMemory.cpp
b/lldb/source/Commands/CommandObjectMemory.cpp
index f13f749da2d21..7005db3088069 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -1643,19 +1643,113 @@ class CommandObjectMemoryHistory : public
CommandObjectParsed {
// CommandObjectMemoryRegion
#pragma mark CommandObjectMemoryRegion
+#define LLDB_OPTIONS_memory_region
+#include "CommandOptions.inc"
+
class CommandObjectMemoryRegion : public CommandObjectParsed {
public:
+ class OptionGroupMemoryRegion : public OptionGroup {
+ public:
+OptionGroupMemoryRegion() : OptionGroup(), m_all(false, false) {}
+
+~OptionGroupMemoryRegion() override = default;
+
+llvm::ArrayRef GetDefinitions() override {
+ return llvm::makeArrayRef(g_memory_region_options);
+}
+
+Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
+ ExecutionContext *execution_context) override {
+ Status status;
+ const int short_option =
g_memory_region_options[option_idx].short_option;
+
+ switch (short_option) {
+ case 'a':
+m_all.SetCurrentValue(true);
+m_all.SetOptionWasSet();
+break;
+ default:
+llvm_unreachable("Unimplemented option");
+ }
+
+ return status;
+}
+
+void OptionParsingStarting(ExecutionContext *execution_context) override {
+ m_all.Clear();
+}
+
+OptionValueBoolean m_all;
+ };
+
CommandObjectMemoryRegion(CommandInterpreter &interpreter)
: CommandObjectParsed(interpreter, "memory region",
"Get information on the memory region containing "
"an address in the current target process.",
-"memory region ADDR",
+"memory region (or --all)",
eCommandRequiresProcess | eCommandTryTargetAPILock
|
-eCommandProcessMustBeLaunched) {}
+eCommandProcessMustBeLaunched) {
+// Address in option set 1.
+m_arguments.push_back(CommandArgumentEntry{CommandArgumentData(
+eArgTypeAddressOrExpression, eArgRepeatPlain, LLDB_OPT_SET_1)});
+// "--all" will go in option set 2.
+m_option_group.Append(&m_memory_region_options);
+m_option_group.Finalize();
+ }
~CommandObjectMemoryRegion() override = default;
+ Options *GetOptions() override { return &m_option_group; }
+
protected:
+ void DumpRegion(CommandReturnObject &result, Target &target,
+ const MemoryRegionInfo &range_info, lldb::addr_t load_addr) {
+lldb_private::Address addr;
+ConstString section_name;
+if (target.ResolveLoadAddress(load_addr, addr)) {
+ SectionSP section_sp(addr.GetSection());
+ if (section_sp) {
+// Got the top most section, not the deepest section
+while (section_sp->GetParent())
+ section_sp = section_sp->GetParent();
+section_name = sec
[Lldb-commits] [PATCH] D111791: [lldb] Add --all option to "memory region"
DavidSpickett added a comment. Relanded in https://reviews.llvm.org/rG13e1cf806567bc4987817e14a774198c3e3f2709. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111791/new/ https://reviews.llvm.org/D111791 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D125090: [lldb] Add --show-tags option to "memory find"
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG068f14f1e4ec: [lldb] Add --show-tags option to "memory
find" (authored by DavidSpickett).
Changed prior to commit:
https://reviews.llvm.org/D125090?vs=427612&id=430654#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125090/new/
https://reviews.llvm.org/D125090
Files:
lldb/include/lldb/Interpreter/OptionGroupMemoryTag.h
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Interpreter/OptionGroupMemoryTag.cpp
lldb/test/API/commands/help/TestHelp.py
lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
lldb/test/API/linux/aarch64/mte_tag_access/main.c
llvm/docs/ReleaseNotes.rst
Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -176,6 +176,9 @@
memory regions (including unmapped ranges). This is the equivalent
of using address 0 then repeating the command until all regions
have been listed.
+* Added "--show-tags" option to the "memory find" command. This is off by default.
+ When enabled, if the target value is found in tagged memory, the tags for that
+ memory will be shown inline with the memory contents.
Changes to Sanitizers
-
Index: lldb/test/API/linux/aarch64/mte_tag_access/main.c
===
--- lldb/test/API/linux/aarch64/mte_tag_access/main.c
+++ lldb/test/API/linux/aarch64/mte_tag_access/main.c
@@ -2,6 +2,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -53,6 +54,9 @@
char *non_mte_buf = checked_mmap(page_size, PROT_READ);
char *mte_read_only = checked_mmap(page_size, mte_prot);
+ // Target value for "memory find" testing.
+ strncpy(mte_buf+128, "LLDB", 4);
+
// Set incrementing tags until end of the first page
char *tagged_ptr = mte_buf;
// This ignores tag bits when subtracting the addresses
Index: lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
===
--- lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
+++ lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
@@ -441,3 +441,34 @@
# A fresh command reverts to the default of tags being off.
self.expect("memory read mte_buf mte_buf+16 -f \"x\"",
patterns=["0x[0-9A-fa-f]+00: 0x0+ 0x0+ 0x0+ 0x0+"])
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+@skipUnlessAArch64MTELinuxCompiler
+def test_mte_memory_find(self):
+"""Test the --show-tags option with memory find."""
+self.setup_mte_test()
+
+# No result, nothing changes.
+self.expect("memory find -s \"foo\" mte_buf mte_buf+32 --show-tags",
+substrs=["data not found within the range."])
+
+cmd = "memory find -s \"LLDB\" mte_buf+64 mte_buf+512"
+found_pattern = "data found at location: 0x[0-9A-Fa-f]+80"
+results_patterns = [
+"0x[0-9A-Fa-f]+80: 4c 4c 44 42 (00 )+ LLDB\.+",
+"0x[0-9A-Fa-f]+90: 00 00 00 00 (00 )+ \.+"
+]
+
+# Default is not to show tags
+self.expect(cmd, patterns=[found_pattern, *results_patterns])
+self.expect(cmd + " --show-tags", patterns=[found_pattern,
+results_patterns[0] + " \(tag: 0x8\)",
+results_patterns[1] + " \(tag: 0x9\)"])
+
+# Uses the same logic as memory read to handle misalignment.
+self.expect("memory find -s \"DB\" mte_buf+64 mte_buf+512 --show-tags",
+patterns=[
+"data found at location: 0x[0-9A-Fa-f]+82\n"
+"0x[0-9A-Fa-f]+82: 44 42 (00 )+ DB\.+ \(tags: 0x8 0x9\)\n",
+"0x[0-9A-Fa-f]+92: 00 00 (00 )+ ..\.+ \(tags: 0x9 0xa\)"])
Index: lldb/test/API/commands/help/TestHelp.py
===
--- lldb/test/API/commands/help/TestHelp.py
+++ lldb/test/API/commands/help/TestHelp.py
@@ -303,3 +303,13 @@
self.assertEqual(sorted(short_options), short_options,
"Short option help displayed in an incorrect order!")
+
+@no_debug_info_test
+def test_help_show_tags(self):
+""" Check that memory find and memory read have the --show-tags option
+but only memory read mentions binary output. """
+self.expect("help memory read", patterns=[
+"--show-tags\n\s+Include memory tags in output "
+"\(does not apply to binary output\)."])
+self.expect("help memory find", patterns=[
+"--show-tags\n\s+Include memory tags in output."])
Index: lldb/source/
[Lldb-commits] [lldb] 068f14f - [lldb] Add --show-tags option to "memory find"
Author: David Spickett
Date: 2022-05-19T14:40:01+01:00
New Revision: 068f14f1e4ec69d218df544487f9420f2b3ab29b
URL:
https://github.com/llvm/llvm-project/commit/068f14f1e4ec69d218df544487f9420f2b3ab29b
DIFF:
https://github.com/llvm/llvm-project/commit/068f14f1e4ec69d218df544487f9420f2b3ab29b.diff
LOG: [lldb] Add --show-tags option to "memory find"
This is off by default. If you get a result and that
memory has memory tags, when --show-tags is given you'll
see the tags inline with the memory content.
```
(lldb) memory read mte_buf mte_buf+64 --show-tags
<...>
0xf7ff8020: 00 00 00 00 00 00 00 00 0d f0 fe ca 00 00 00 00
(tag: 0x2)
<...>
(lldb) memory find -e 0xcafef00d mte_buf mte_buf+64 --show-tags
data found at location: 0xf7ff8028
0xf7ff8028: 0d f0 fe ca 00 00 00 00 00 00 00 00 00 00 00 00
(tags: 0x2 0x3)
0xf7ff8038: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
(tags: 0x3 0x4)
```
The logic for handling alignments is the same as for memory read
so in the above example because the line starts misaligned to the
granule it covers 2 granules.
Depends on D125089
Reviewed By: omjavaid
Differential Revision: https://reviews.llvm.org/D125090
Added:
Modified:
lldb/include/lldb/Interpreter/OptionGroupMemoryTag.h
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Interpreter/OptionGroupMemoryTag.cpp
lldb/test/API/commands/help/TestHelp.py
lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
lldb/test/API/linux/aarch64/mte_tag_access/main.c
llvm/docs/ReleaseNotes.rst
Removed:
diff --git a/lldb/include/lldb/Interpreter/OptionGroupMemoryTag.h
b/lldb/include/lldb/Interpreter/OptionGroupMemoryTag.h
index 956ec3f07a9b..918ea3ad96df 100644
--- a/lldb/include/lldb/Interpreter/OptionGroupMemoryTag.h
+++ b/lldb/include/lldb/Interpreter/OptionGroupMemoryTag.h
@@ -16,7 +16,10 @@ namespace lldb_private {
class OptionGroupMemoryTag : public OptionGroup {
public:
- OptionGroupMemoryTag();
+ OptionGroupMemoryTag(
+ // Whether to note that --show-tags does not apply to binary output.
+ // "memory read" wants this but "memory find" does not.
+ bool note_binary = false);
~OptionGroupMemoryTag() override = default;
@@ -33,6 +36,7 @@ class OptionGroupMemoryTag : public OptionGroup {
protected:
OptionValueBoolean m_show_tags;
+ OptionDefinition m_option_definition;
};
} // namespace lldb_private
diff --git a/lldb/source/Commands/CommandObjectMemory.cpp
b/lldb/source/Commands/CommandObjectMemory.cpp
index 7005db308806..b7678add5399 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -288,7 +288,7 @@ class CommandObjectMemoryRead : public CommandObjectParsed {
"Read from the memory of the current target process.", nullptr,
eCommandRequiresTarget | eCommandProcessMustBePaused),
m_format_options(eFormatBytesWithASCII, 1, 8),
-
+m_memory_tag_options(/*note_binary=*/true),
m_prev_format_options(eFormatBytesWithASCII, 1, 8) {
CommandArgumentEntry arg1;
CommandArgumentEntry arg2;
@@ -975,6 +975,8 @@ class CommandObjectMemoryFind : public CommandObjectParsed {
m_arguments.push_back(arg2);
m_option_group.Append(&m_memory_options);
+m_option_group.Append(&m_memory_tag_options, LLDB_OPT_SET_ALL,
+ LLDB_OPT_SET_ALL);
m_option_group.Finalize();
}
@@ -1139,7 +1141,9 @@ class CommandObjectMemoryFind : public
CommandObjectParsed {
DumpDataExtractor(
data, &result.GetOutputStream(), 0, lldb::eFormatBytesWithASCII, 1,
dumpbuffer.GetByteSize(), 16,
-found_location + m_memory_options.m_offset.GetCurrentValue(), 0,
0);
+found_location + m_memory_options.m_offset.GetCurrentValue(), 0, 0,
+m_exe_ctx.GetBestExecutionContextScope(),
+m_memory_tag_options.GetShowTags().GetCurrentValue());
result.GetOutputStream().EOL();
}
@@ -1182,6 +1186,7 @@ class CommandObjectMemoryFind : public
CommandObjectParsed {
OptionGroupOptions m_option_group;
OptionGroupFindMemory m_memory_options;
+ OptionGroupMemoryTag m_memory_tag_options;
};
#define LLDB_OPTIONS_memory_write
diff --git a/lldb/source/Interpreter/OptionGroupMemoryTag.cpp
b/lldb/source/Interpreter/OptionGroupMemoryTag.cpp
index f63dfd6bcac6..6752b6c8acf2 100644
--- a/lldb/source/Interpreter/OptionGroupMemoryTag.cpp
+++ b/lldb/source/Interpreter/OptionGroupMemoryTag.cpp
@@ -13,25 +13,26 @@
using namespace lldb;
using namespace lldb_private;
-OptionGroupMemoryTag::OptionGroupMemoryTag() : m_show_tags(false, false) {}
-
static const uint32_t SHORT_OPTION_SHOW_TAGS = 0x54414753; // 'tags'
-static constexpr OptionDefinition g_option_table[] = {
-
[Lldb-commits] [PATCH] D125503: [trace][intelpt] Support system-wide tracing [7] - Create a base IntelPTProcessTrace class
jj10306 requested changes to this revision.
jj10306 added inline comments.
This revision now requires changes to proceed.
Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.h:73
+ /// The target process.
NativeProcessProtocol &m_process;
/// Threads traced due to "thread tracing"
Could this be moved to be a part of the new `IntelPTProcessTrace` abstract
class or is this also needed for handling `thread trace`?
Comment at: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp:36
-Expected IntelPTMultiCoreTrace::StartOnAllCores(
-const TraceIntelPTStartRequest &request) {
+Expected
+IntelPTMultiCoreTrace::StartOnAllCores(const TraceIntelPTStartRequest &request,
If this now returns `IntelPTProcessTraceUP` instead of an instance of itself,
then we are basically forcing any uses of this class to be done using dynamic
polymorphism (behind the indirection of a pointer to the super class). Should
this instead return an instance of itself and then leave it up to the caller to
decide if they want to use the object directly or behind a pointer?
I know that currently the only use of this is behind the
`IntelPTProcessTraceUP` (stored on the collector), but maybe in the future we
would want to use it directly.
wdyt?
Comment at: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp:119-125
+ // All the process' threads are being traced automatically.
+ return (bool)m_process.GetThreadByID(tid);
+}
+
+llvm::Error IntelPTMultiCoreTrace::TraceStart(lldb::tid_t tid) {
+ // This instance is already tracing all threads automatically.
+ return llvm::Error::success();
In `TracesThread` you check that the tid is a thread of `m_process` but in
`TraceStart` you return success for all threads without checking if it's a part
of the process.
I don't think it particularly matters if we do the check or not, but I do think
that the behavior should be consistent between these functions.
Comment at:
lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.cpp:58
+
+Expected
+IntelPTPerThreadProcessTrace::Start(const TraceIntelPTStartRequest &request,
same comment here as above on the other subclasss
Comment at: lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h:19
+
+// Abstract class to be inherited by all the process tracing strategies.
+class IntelPTProcessTrace {
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125503/new/
https://reviews.llvm.org/D125503
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D125850: [trace][intelpt] Support system-wide tracing [8] - Improve the single buffer perf_event configuration
jj10306 requested changes to this revision.
jj10306 added inline comments.
This revision now requires changes to proceed.
Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp:186
attr.exclude_kernel = 1;
- attr.sample_type = PERF_SAMPLE_TIME;
- attr.sample_id_all = 1;
won't we need this in order to get timestamps in the context switching events?
I agree we don't need it for the time being so maybe in the diff where you add
context switch collection you will reintroduce it 🙂
Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp:316
-if (Error mmap_err = perf_event->MmapMetadataAndBuffers(buffer_numpages,
-buffer_numpages)) {
return std::move(mmap_err);
In the future if we need to collect itrace we will need a small data buffer.
Do you plan to collect context switch info as part of the same perf event used
for intel pt or will you open a separate event?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125850/new/
https://reviews.llvm.org/D125850
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D125897: [trace][intelpt] Support system-wide tracing [9] - Collect and return context switch traces
jj10306 requested changes to this revision. jj10306 added a comment. This revision now requires changes to proceed. Submitting my comments so far, will continue my review in a bit Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:26-28 static const char *kProcFsCpuInfo; static const char *kTraceBuffer; + static const char *kPerfContextSwitchTrace; nit: these are just pointers to a buffer of bytes right? if so, maybe we could change the types to be `const uint8_t *`. Obviously it doesn't make a difference since they are the same size, but when I see `const char *` my brain kinda immediately thinks STRING! maybe it's just me though 🤩, wdyt? Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.h:41-45 + void ProcessDidStop(); + + /// To be invoked before the process will resume, so that we can capture the + /// first instructions after the resume. + void ProcessWillResume(); Why do we need to separate these out into two separate functions? Comment at: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp:36 +static Expected CreateContextSwitchTracePerfEvent( +bool disabled, lldb::core_id_t core_id, thoughts on moving this to be a "utility" method of the Perf.cpp and then call that utility method from here? Moving it there would make it more accessible to other callers. Comment at: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp:47 + attr.size = sizeof(attr); + attr.sample_period = 0; + attr.sample_type = PERF_SAMPLE_TID | PERF_SAMPLE_TIME; what does this do? Comment at: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp:51-54 + attr.exclude_kernel = 1; + attr.sample_id_all = 1; + attr.exclude_hv = 1; + attr.disabled = disabled; In addition to this context switching event needing to be part of the same group, you also must ensure that some other config bits (exclude_kernel, exclude_hv) are the same. Also, what would happen if the disabled bit is set here but then the enabled bit of the intel pt event was set? All of these considerations related to keeping the two events "in sync", are beginning to make me lean towards what I mentioned above about using the same perf event, because that would naturally remove any opportunities for the two events to be "out of sync" Comment at: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp:56 + + // The given perf configuration will product context switch records of 32 + // bytes each. Assuming that every context switch will be emitted twice (one Comment at: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp:101-109 +Expected core_trace = +IntelPTSingleBufferTrace::Start(request, /*tid=*/None, core_id, +/*disabled=*/true); +if (!core_trace) return IncludePerfEventParanoidMessageInError(core_trace.takeError()); + +if (Expected context_switch_trace = to reduce opportunity for things to get out of sync if this is ever touched in the future? Comment at: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h:97 + llvm::DenseMap> + m_traces_per_core; perhaps you could make this it's own structure? that would make things a bit less verbose here and in the ForEachCore callback? also, this may make it clearer that they must be part of the same perf event and potentially would allow enforcing this easier? Instead of creating an entirely new structure you also could make the ContextSwitchTrace an optional member of IntelPTSingleBuffer trace, but not sure if that's the best place to do the coupling. Another option would be to just use the same underlying perf_event of IntelPTSingleBufferTrace. That would naturally enforce the constraint of the events being part of the same group and wouldn't require adding this new ContextSwitchTrace notion. lots of options lol, I think things are fine as is but wanted to share my thoughts. wdyt? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125897/new/ https://reviews.llvm.org/D125897 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D125995: [lldb] Fix 'ptsname_r' is only available on macOS 10.13.4 or newer
JDevlieghere created this revision.
JDevlieghere added reviewers: mib, aprantl.
Herald added a project: All.
JDevlieghere requested review of this revision.
Herald added a project: LLDB.
A deployment target that's less than 10.13.4 causes an error saying that
'ptsname_r' is only available on macOS 10.13.4 or newer. The current logic only
checks if the symbol is available and doesn't account for the deployment
target. This patch fixes that.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D125995
Files:
lldb/source/Host/common/PseudoTerminal.cpp
Index: lldb/source/Host/common/PseudoTerminal.cpp
===
--- lldb/source/Host/common/PseudoTerminal.cpp
+++ lldb/source/Host/common/PseudoTerminal.cpp
@@ -22,6 +22,10 @@
#include "lldb/Host/PosixApi.h"
+#if defined(__APPLE__)
+#include
+#endif
+
#if defined(__ANDROID__)
int posix_openpt(int flags);
#endif
@@ -99,21 +103,33 @@
std::error_code(errno, std::generic_category()));
}
-std::string PseudoTerminal::GetSecondaryName() const {
- assert(m_primary_fd >= 0);
-#if HAVE_PTSNAME_R
- char buf[PATH_MAX];
- buf[0] = '\0';
- int r = ptsname_r(m_primary_fd, buf, sizeof(buf));
- (void)r;
- assert(r == 0);
- return buf;
-#else
+static std::string use_ptsname(int fd) {
static std::mutex mutex;
std::lock_guard guard(mutex);
- const char *r = ptsname(m_primary_fd);
+ const char *r = ptsname(fd);
assert(r != nullptr);
return r;
+}
+
+std::string PseudoTerminal::GetSecondaryName() const {
+ assert(m_primary_fd >= 0);
+#if HAVE_PTSNAME_R
+#if defined(__APPLE__)
+ if (__builtin_available(macos 10.13.4, iOS 11.3, tvOS 11.3, watchOS 4.4, *))
{
+#endif
+char buf[PATH_MAX];
+buf[0] = '\0';
+int r = ptsname_r(m_primary_fd, buf, sizeof(buf));
+(void)r;
+assert(r == 0);
+return buf;
+#if defined(__APPLE__)
+ } else {
+return use_ptsname(m_primary_fd);
+ }
+#endif
+#else
+ return use_ptsname(m_primary_fd);
#endif
}
Index: lldb/source/Host/common/PseudoTerminal.cpp
===
--- lldb/source/Host/common/PseudoTerminal.cpp
+++ lldb/source/Host/common/PseudoTerminal.cpp
@@ -22,6 +22,10 @@
#include "lldb/Host/PosixApi.h"
+#if defined(__APPLE__)
+#include
+#endif
+
#if defined(__ANDROID__)
int posix_openpt(int flags);
#endif
@@ -99,21 +103,33 @@
std::error_code(errno, std::generic_category()));
}
-std::string PseudoTerminal::GetSecondaryName() const {
- assert(m_primary_fd >= 0);
-#if HAVE_PTSNAME_R
- char buf[PATH_MAX];
- buf[0] = '\0';
- int r = ptsname_r(m_primary_fd, buf, sizeof(buf));
- (void)r;
- assert(r == 0);
- return buf;
-#else
+static std::string use_ptsname(int fd) {
static std::mutex mutex;
std::lock_guard guard(mutex);
- const char *r = ptsname(m_primary_fd);
+ const char *r = ptsname(fd);
assert(r != nullptr);
return r;
+}
+
+std::string PseudoTerminal::GetSecondaryName() const {
+ assert(m_primary_fd >= 0);
+#if HAVE_PTSNAME_R
+#if defined(__APPLE__)
+ if (__builtin_available(macos 10.13.4, iOS 11.3, tvOS 11.3, watchOS 4.4, *)) {
+#endif
+char buf[PATH_MAX];
+buf[0] = '\0';
+int r = ptsname_r(m_primary_fd, buf, sizeof(buf));
+(void)r;
+assert(r == 0);
+return buf;
+#if defined(__APPLE__)
+ } else {
+return use_ptsname(m_primary_fd);
+ }
+#endif
+#else
+ return use_ptsname(m_primary_fd);
#endif
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D125995: [lldb] Fix 'ptsname_r' is only available on macOS 10.13.4 or newer
mib accepted this revision. mib added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125995/new/ https://reviews.llvm.org/D125995 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort
hawkinsw updated this revision to Diff 430781.
hawkinsw added a comment.
Stupidly missed a few of the places where bare assertions were used.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124731/new/
https://reviews.llvm.org/D124731
Files:
lldb/source/Breakpoint/BreakpointResolverAddress.cpp
lldb/source/Commands/Options.td
lldb/test/API/commands/breakpoint/set/address-nomodule/Makefile
lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py
lldb/test/API/commands/breakpoint/set/address-nomodule/inferior.c
Index: lldb/test/API/commands/breakpoint/set/address-nomodule/inferior.c
===
--- /dev/null
+++ lldb/test/API/commands/breakpoint/set/address-nomodule/inferior.c
@@ -0,0 +1,6 @@
+int function(int a) { return a; }
+
+int main() {
+ int f = function(10);
+ return f;
+}
Index: lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py
===
--- /dev/null
+++ lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py
@@ -0,0 +1,36 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def get_address_from_symbol(self, symbol):
+target = lldbutil.run_to_breakpoint_make_target(self, "a.out", True)
+bp = target.BreakpointCreateByName(symbol, None)
+address = bp.GetLocationAtIndex(0).GetAddress().GetFileAddress()
+return address
+
+def test_set_address_no_module(self):
+self.build()
+
+main_address = self.get_address_from_symbol("main")
+
+target = lldbutil.run_to_breakpoint_make_target(self)
+debugger = target.GetDebugger()
+
+debugger.HandleCommand(f"break set -a {main_address:#x}")
+self.assertEquals(target.GetNumBreakpoints(), 1)
+
+bp = target.GetBreakpointAtIndex(0)
+self.assertIsNotNone(bp)
+
+_, _, thread, _ = lldbutil.run_to_breakpoint_do_run(self, target, bp)
+self.assertGreaterEqual(thread.GetNumFrames(), 1)
+
+thread_pc = thread.GetFrameAtIndex(0).GetPCAddress()
+self.assertNotEqual(thread_pc, None)
+
+self.assertEquals(main_address, thread_pc.GetFileAddress())
Index: lldb/test/API/commands/breakpoint/set/address-nomodule/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/breakpoint/set/address-nomodule/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := inferior.c
+
+include Makefile.rules
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -128,13 +128,15 @@
Arg<"AddressOrExpression">, Required,
Desc<"Set the breakpoint at the specified address. If the address maps "
"uniquely to a particular binary, then the address will be converted to "
-"a \"file\"address, so that the breakpoint will track that binary+offset "
+"a \"file\" address, so that the breakpoint will track that binary+offset "
"no matter where the binary eventually loads. Alternately, if you also "
"specify the module - with the -s option - then the address will be "
"treated as a file address in that module, and resolved accordingly. "
"Again, this will allow lldb to track that offset on subsequent reloads. "
"The module need not have been loaded at the time you specify this "
-"breakpoint, and will get resolved when the module is loaded.">;
+"breakpoint, and will get resolved when the module is loaded. If no "
+"module is specified, the binary being debugged is considered as a "
+"fallback.">;
def breakpoint_set_name : Option<"name", "n">, Group<3>, Arg<"FunctionName">,
Completion<"Symbol">, Required,
Desc<"Set the breakpoint by function name. Can be repeated multiple times "
Index: lldb/source/Breakpoint/BreakpointResolverAddress.cpp
===
--- lldb/source/Breakpoint/BreakpointResolverAddress.cpp
+++ lldb/source/Breakpoint/BreakpointResolverAddress.cpp
@@ -121,16 +121,27 @@
if (filter.AddressPasses(m_addr)) {
if (breakpoint.GetNumLocations() == 0) {
- // If the address is just an offset, and we're given a module, see if we
- // can find the appropriate module loaded in the binary, and fix up
- // m_addr to use that.
- if (!m_addr.IsSectionOffset() && m_module_filespec) {
+ // If the address is just an offset ...
+ if (!m_addr.IsSectionOffset()) {
+ModuleSP containing_module_sp = nullptr;
Target &target = breakpoint.GetTarget();
-ModuleSpec module_spec(m_module_filespec);
-
[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort
hawkinsw marked 2 inline comments as done. hawkinsw added a comment. @JDevlieghere So sorry! I cannot believe I missed those. Thanks for putting up with me! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124731/new/ https://reviews.llvm.org/D124731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D126013: Add [opt] suffix to optimized stack frame in lldb-vscode
yinghuitan created this revision.
yinghuitan added reviewers: clayborg, labath, jingham, jdoerfert, JDevlieghere.
Herald added a project: All.
yinghuitan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
To help user identify optimized code This diff adds a "[opt]" suffix to
optimized stack frames in lldb-vscode. This provides consistent experience
as command line lldb.
It also adds a new "optimized" attribute to DAP stack frame object so that
it is easy to identify from telemetry than parsing trailing "[opt]".
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D126013
Files:
lldb/test/API/tools/lldb-vscode/optimized/Makefile
lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
lldb/test/API/tools/lldb-vscode/optimized/main.cpp
lldb/tools/lldb-vscode/JSONUtils.cpp
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -751,7 +751,18 @@
llvm::json::Object object;
int64_t frame_id = MakeVSCodeFrameID(frame);
object.try_emplace("id", frame_id);
- EmplaceSafeString(object, "name", frame.GetFunctionName());
+ bool is_optimized =
+ frame.GetFunction().IsValid() && frame.GetFunction().GetIsOptimized();
+ if (is_optimized) {
+std::string frame_name;
+frame_name += frame.GetFunctionName();
+frame_name += " [opt]";
+EmplaceSafeString(object, "name", frame_name);
+ } else {
+EmplaceSafeString(object, "name", frame.GetFunctionName());
+ }
+ object.try_emplace("optimized", is_optimized);
+
int64_t disasm_line = 0;
object.try_emplace("source", CreateSource(frame, disasm_line));
Index: lldb/test/API/tools/lldb-vscode/optimized/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/optimized/main.cpp
@@ -0,0 +1,16 @@
+#include
+#include
+
+int foo(int x, int y) {
+ printf("Got input %d, %d\n", x, y);
+ return x + y + 3; // breakpoint 1
+}
+
+int main(int argc, char const *argv[]) {
+ int optimized = argc > 1 ? std::stoi(argv[1]) : 0;
+
+ printf("argc: %d, optimized: %d\n", argc, optimized);
+ int result = foo(argc, 20);
+ printf("result: %d\n", result); // breakpoint 2
+ return 0;
+}
Index: lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
@@ -0,0 +1,35 @@
+"""
+Test lldb-vscode variables/stackTrace request for optimized code
+"""
+
+from __future__ import print_function
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+
+
+class TestVSCode_optimized(lldbvscode_testcase.VSCodeTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfWindows
+@skipIfRemote
+def test_stack_frame_name(self):
+''' Test optimized frame has special name suffix.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = 'main.cpp'
+breakpoint_line = line_number(source, '// breakpoint 1')
+lines = [breakpoint_line]
+breakpoint_ids = self.set_source_breakpoints(source, lines)
+self.assertEqual(len(breakpoint_ids), len(lines),
+"expect correct number of breakpoints")
+self.continue_to_breakpoints(breakpoint_ids)
+leaf_frame = self.vscode.get_stackFrame(frameIndex=0)
+self.assertTrue(leaf_frame['name'].endswith(' [opt]'))
+parent_frame = self.vscode.get_stackFrame(frameIndex=1)
+self.assertTrue(parent_frame['name'].endswith(' [opt]'))
Index: lldb/test/API/tools/lldb-vscode/optimized/Makefile
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/optimized/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -O3 -glldb
+include Makefile.rules
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -751,7 +751,18 @@
llvm::json::Object object;
int64_t frame_id = MakeVSCodeFrameID(frame);
object.try_emplace("id", frame_id);
- EmplaceSafeString(object, "name", frame.GetFunctionName());
+ bool is_optimized =
+ frame.GetFunction().IsValid() && frame.GetFunction().GetIsOptimized();
+ if (is_optimized) {
+std::string frame_name;
+frame_name += frame.GetFunctionName();
+frame_name += " [opt]";
+EmplaceSafeString(object, "name", frame_name);
+ } else {
+EmplaceSafeString(object, "name", frame.GetFunctionName());
+ }
+ object.try_emplace("optimized", is_optimize
[Lldb-commits] [PATCH] D126014: Show error message for optimized variables
yinghuitan created this revision.
yinghuitan added reviewers: clayborg, labath, jingham, jdoerfert, JDevlieghere,
kusmour, aadsm.
Herald added a project: All.
yinghuitan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
This fixes an issue that optimized variable error message is not shown to end
users in lldb-vscode.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D126014
Files:
lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
lldb/tools/lldb-vscode/JSONUtils.cpp
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -136,10 +136,13 @@
llvm::StringRef value = v.GetValue();
llvm::StringRef summary = v.GetSummary();
llvm::StringRef type_name = v.GetType().GetDisplayTypeName();
+ lldb::SBError error = v.GetError();
std::string result;
llvm::raw_string_ostream strm(result);
- if (!value.empty()) {
+ if (!error.Success()) {
+strm << "";
+ } else if (!value.empty()) {
strm << value;
if (!summary.empty())
strm << ' ' << summary;
Index: lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
===
--- lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
+++ lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
@@ -33,3 +33,22 @@
self.assertTrue(leaf_frame['name'].endswith(' [opt]'))
parent_frame = self.vscode.get_stackFrame(frameIndex=1)
self.assertTrue(parent_frame['name'].endswith(' [opt]'))
+
+@skipIfWindows
+@skipIfRemote
+def test_optimized_variable(self):
+''' Test optimized variable value contains error.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = 'main.cpp'
+breakpoint_line = line_number(source, '// breakpoint 2')
+lines = [breakpoint_line]
+# Set breakpoint in the thread function so we can step the threads
+breakpoint_ids = self.set_source_breakpoints(source, lines)
+self.assertEqual(len(breakpoint_ids), len(lines),
+"expect correct number of breakpoints")
+self.continue_to_breakpoints(breakpoint_ids)
+optimized_variable = self.vscode.get_local_variable('optimized')
+
+self.assertTrue(optimized_variable['value'].startswith('Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -136,10 +136,13 @@
llvm::StringRef value = v.GetValue();
llvm::StringRef summary = v.GetSummary();
llvm::StringRef type_name = v.GetType().GetDisplayTypeName();
+ lldb::SBError error = v.GetError();
std::string result;
llvm::raw_string_ostream strm(result);
- if (!value.empty()) {
+ if (!error.Success()) {
+strm << "";
+ } else if (!value.empty()) {
strm << value;
if (!summary.empty())
strm << ' ' << summary;
Index: lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
===
--- lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
+++ lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
@@ -33,3 +33,22 @@
self.assertTrue(leaf_frame['name'].endswith(' [opt]'))
parent_frame = self.vscode.get_stackFrame(frameIndex=1)
self.assertTrue(parent_frame['name'].endswith(' [opt]'))
+
+@skipIfWindows
+@skipIfRemote
+def test_optimized_variable(self):
+''' Test optimized variable value contains error.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = 'main.cpp'
+breakpoint_line = line_number(source, '// breakpoint 2')
+lines = [breakpoint_line]
+# Set breakpoint in the thread function so we can step the threads
+breakpoint_ids = self.set_source_breakpoints(source, lines)
+self.assertEqual(len(breakpoint_ids), len(lines),
+"expect correct number of breakpoints")
+self.continue_to_breakpoints(breakpoint_ids)
+optimized_variable = self.vscode.get_local_variable('optimized')
+
+self.assertTrue(optimized_variable['value'].startswith('___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D126014: Show error message for optimized variables
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. A little background: we have people that end up debugging optimized code and when a variable's SBValue had an error, we wouldn't show anything in the variables window for the value. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126014/new/ https://reviews.llvm.org/D126014 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D126013: Add [opt] suffix to optimized stack frame in lldb-vscode
clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:754-755
object.try_emplace("id", frame_id);
- EmplaceSafeString(object, "name", frame.GetFunctionName());
+ bool is_optimized =
+ frame.GetFunction().IsValid() && frame.GetFunction().GetIsOptimized();
+ if (is_optimized) {
No need to test if the function is valid, the SBFunction::IsValid() does this
already for us and will return false if it isn't valid.
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:758
+std::string frame_name;
+frame_name += frame.GetFunctionName();
+frame_name += " [opt]";
This will crash if the function name is NULL, so we should guard against this.
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:762
+ } else {
+EmplaceSafeString(object, "name", frame.GetFunctionName());
+ }
We should probably get the function name with "const char *func_name =
frame.GetFunctionName();" before the entire "if (is_optimized)" and use it
here, and above
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126013/new/
https://reviews.llvm.org/D126013
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D126013: Add [opt] suffix to optimized stack frame in lldb-vscode
clayborg added inline comments.
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:754-755
object.try_emplace("id", frame_id);
- EmplaceSafeString(object, "name", frame.GetFunctionName());
+ bool is_optimized =
+ frame.GetFunction().IsValid() && frame.GetFunction().GetIsOptimized();
+ if (is_optimized) {
clayborg wrote:
> No need to test if the function is valid, the SBFunction::IsValid() does this
> already for us and will return false if it isn't valid.
I meant to say "the SBFunction::GetIsOptimized()" does this already and will
return false if it isn't valid
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126013/new/
https://reviews.llvm.org/D126013
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D126021: [lldb/test] Fix PExpect.launch issue when disabling color support
mib created this revision.
mib added a reviewer: JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
This patch should fix a bug in PExpect.launch that happened when color
support is not enabled.
In that case, we need to add the `--no-use-colors` flag to lldb's launch
argument list. However, previously, each character to the string was
appended separately to the `args` list. This patch solves that by adding
the whole string to the list.
This should fix the TestIOHandlerResize failure on GreenDragon.
Signed-off-by: Med Ismail Bennani
https://reviews.llvm.org/D126021
Files:
lldb/packages/Python/lldbsuite/test/lldbpexpect.py
Index: lldb/packages/Python/lldbsuite/test/lldbpexpect.py
===
--- lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -34,7 +34,7 @@
args += run_under
args += [lldbtest_config.lldbExec, '--no-lldbinit']
if not use_colors:
-args += '--no-use-colors'
+args.append('--no-use-colors')
for cmd in self.setUpCommands():
if "use-color false" in cmd and use_colors:
continue
Index: lldb/packages/Python/lldbsuite/test/lldbpexpect.py
===
--- lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -34,7 +34,7 @@
args += run_under
args += [lldbtest_config.lldbExec, '--no-lldbinit']
if not use_colors:
-args += '--no-use-colors'
+args.append('--no-use-colors')
for cmd in self.setUpCommands():
if "use-color false" in cmd and use_colors:
continue
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 027499a - [lldb/test] Fix PExpect.launch issue when disabling color support
Author: Med Ismail Bennani
Date: 2022-05-19T14:47:04-07:00
New Revision: 027499a82434ea7a784b2e5a0227540f00fbc307
URL:
https://github.com/llvm/llvm-project/commit/027499a82434ea7a784b2e5a0227540f00fbc307
DIFF:
https://github.com/llvm/llvm-project/commit/027499a82434ea7a784b2e5a0227540f00fbc307.diff
LOG: [lldb/test] Fix PExpect.launch issue when disabling color support
This patch should fix a bug in PExpect.launch that happened when color
support is not enabled.
In that case, we need to add the `--no-use-colors` flag to lldb's launch
argument list. However, previously, each character to the string was
appended separately to the `args` list. This patch solves that by adding
the whole string to the list.
This should fix the TestIOHandlerResize failure on GreenDragon.
Differential Revision: https://reviews.llvm.org/D126021
Signed-off-by: Med Ismail Bennani
Added:
Modified:
lldb/packages/Python/lldbsuite/test/lldbpexpect.py
Removed:
diff --git a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
index 1083705a20119..22a30c5d22c45 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -34,7 +34,7 @@ def launch(self, executable=None, extra_args=None, timeout=60,
args += run_under
args += [lldbtest_config.lldbExec, '--no-lldbinit']
if not use_colors:
-args += '--no-use-colors'
+args.append('--no-use-colors')
for cmd in self.setUpCommands():
if "use-color false" in cmd and use_colors:
continue
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D126021: [lldb/test] Fix PExpect.launch issue when disabling color support
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG027499a82434: [lldb/test] Fix PExpect.launch issue when
disabling color support (authored by mib).
Herald added a subscriber: lldb-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126021/new/
https://reviews.llvm.org/D126021
Files:
lldb/packages/Python/lldbsuite/test/lldbpexpect.py
Index: lldb/packages/Python/lldbsuite/test/lldbpexpect.py
===
--- lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -34,7 +34,7 @@
args += run_under
args += [lldbtest_config.lldbExec, '--no-lldbinit']
if not use_colors:
-args += '--no-use-colors'
+args.append('--no-use-colors')
for cmd in self.setUpCommands():
if "use-color false" in cmd and use_colors:
continue
Index: lldb/packages/Python/lldbsuite/test/lldbpexpect.py
===
--- lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -34,7 +34,7 @@
args += run_under
args += [lldbtest_config.lldbExec, '--no-lldbinit']
if not use_colors:
-args += '--no-use-colors'
+args.append('--no-use-colors')
for cmd in self.setUpCommands():
if "use-color false" in cmd and use_colors:
continue
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D125897: [trace][intelpt] Support system-wide tracing [9] - Collect and return context switch traces
jj10306 added inline comments.
Comment at: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp:122-129
void IntelPTMultiCoreTrace::ForEachCore(
std::function
callback) {
for (auto &it : m_traces_per_core)
-callback(it.first, *it.second);
+callback(it.first, it.second.first);
}
Is there a way to consolidate these two methods?
Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h:43
+ /// \param[in] disabled
+ /// Whether to start the tracing paused.
///
This wording is a bit confusing - maybe provide some additional context here
about this flag controlling the whether the perf event is enabled/disabled at
perf_event_open time
Comment at: lldb/source/Plugins/Process/Linux/Perf.cpp:161
if (Expected mmap_metadata_data =
- DoMmap(nullptr, mmap_size, PROT_WRITE, MAP_SHARED, 0,
+ DoMmap(nullptr, mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED, 0,
"metadata and data buffer")) {
curious what adding the PROT_READ does here, given that this code was working
with just PROT_WRITE it seems like PROT_WRITE implies PROT_READ
Comment at: lldb/source/Plugins/Process/Linux/Perf.cpp:226
+Expected>
+PerfEvent::ReadFlushedOutAuxCyclicBuffer(size_t offset, size_t size) {
+ CollectionState previous_state = m_collection_state;
It's worth noting that this function should only be called when the aux buffer
is configured as a ring buffer (ie the aux buffer was mmaped with PROT_READ).
if the aux buffer mmaped with PROT_WRITE then the behavior of the head is
different and the consumer is required to update the aux_tail when reading.
Perhaps we should check this before doing the reading or make it very clear in
the docs that this should only be called on perf events with that specific
configuration
Comment at: lldb/source/Plugins/Process/Linux/Perf.h:101
class PerfEvent {
+ enum class CollectionState {
+Enabled,
is an enum needed for this? Since the perf event will only ever be in two
states, feels like a bool would suffice
Comment at: lldb/source/Plugins/Process/Linux/Perf.h:238
+ llvm::Expected>
+ ReadFlushedOutAuxCyclicBuffer(size_t offset, size_t size);
+
nit: This is wordy. imo the name needs not mention that it's flushed out, just
put that in the docs
Comment at:
lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py:226-229
+self.assertTrue(context_switch_size is not None)
+self.assertTrue(trace_buffer_size is not None)
+if context_switch_size > 0:
+found_non_empty_context_switch = True
nice test!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125897/new/
https://reviews.llvm.org/D125897
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] ea48640 - [lldb] Fix 'ptsname_r' is only available on macOS 10.13.4 or newer
Author: Jonas Devlieghere
Date: 2022-05-19T21:39:52-07:00
New Revision: ea4864007c72bfe1523013e28ceae4e391b66afc
URL:
https://github.com/llvm/llvm-project/commit/ea4864007c72bfe1523013e28ceae4e391b66afc
DIFF:
https://github.com/llvm/llvm-project/commit/ea4864007c72bfe1523013e28ceae4e391b66afc.diff
LOG: [lldb] Fix 'ptsname_r' is only available on macOS 10.13.4 or newer
A deployment target less than 10.13.4 causes an error saying that
'ptsname_r' is only available on macOS 10.13.4 or newer. The current
logic only checks if the symbol is available and doesn't account for the
deployment target. This patch fixes that by adding an availability
check.
Differential revision: https://reviews.llvm.org/D125995
Added:
Modified:
lldb/source/Host/common/PseudoTerminal.cpp
Removed:
diff --git a/lldb/source/Host/common/PseudoTerminal.cpp
b/lldb/source/Host/common/PseudoTerminal.cpp
index dce4c5185c7b6..13c82e8b1ea91 100644
--- a/lldb/source/Host/common/PseudoTerminal.cpp
+++ b/lldb/source/Host/common/PseudoTerminal.cpp
@@ -22,6 +22,10 @@
#include "lldb/Host/PosixApi.h"
+#if defined(__APPLE__)
+#include
+#endif
+
#if defined(__ANDROID__)
int posix_openpt(int flags);
#endif
@@ -99,21 +103,33 @@ llvm::Error PseudoTerminal::OpenSecondary(int oflag) {
std::error_code(errno, std::generic_category()));
}
-std::string PseudoTerminal::GetSecondaryName() const {
- assert(m_primary_fd >= 0);
-#if HAVE_PTSNAME_R
- char buf[PATH_MAX];
- buf[0] = '\0';
- int r = ptsname_r(m_primary_fd, buf, sizeof(buf));
- (void)r;
- assert(r == 0);
- return buf;
-#else
+static std::string use_ptsname(int fd) {
static std::mutex mutex;
std::lock_guard guard(mutex);
- const char *r = ptsname(m_primary_fd);
+ const char *r = ptsname(fd);
assert(r != nullptr);
return r;
+}
+
+std::string PseudoTerminal::GetSecondaryName() const {
+ assert(m_primary_fd >= 0);
+#if HAVE_PTSNAME_R
+#if defined(__APPLE__)
+ if (__builtin_available(macos 10.13.4, iOS 11.3, tvOS 11.3, watchOS 4.4, *))
{
+#endif
+char buf[PATH_MAX];
+buf[0] = '\0';
+int r = ptsname_r(m_primary_fd, buf, sizeof(buf));
+(void)r;
+assert(r == 0);
+return buf;
+#if defined(__APPLE__)
+ } else {
+return use_ptsname(m_primary_fd);
+ }
+#endif
+#else
+ return use_ptsname(m_primary_fd);
#endif
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D125995: [lldb] Fix 'ptsname_r' is only available on macOS 10.13.4 or newer
This revision was automatically updated to reflect the committed changes.
Closed by commit rGea4864007c72: [lldb] Fix 'ptsname_r' is only
available on macOS 10.13.4 or newer (authored by JDevlieghere).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125995/new/
https://reviews.llvm.org/D125995
Files:
lldb/source/Host/common/PseudoTerminal.cpp
Index: lldb/source/Host/common/PseudoTerminal.cpp
===
--- lldb/source/Host/common/PseudoTerminal.cpp
+++ lldb/source/Host/common/PseudoTerminal.cpp
@@ -22,6 +22,10 @@
#include "lldb/Host/PosixApi.h"
+#if defined(__APPLE__)
+#include
+#endif
+
#if defined(__ANDROID__)
int posix_openpt(int flags);
#endif
@@ -99,21 +103,33 @@
std::error_code(errno, std::generic_category()));
}
-std::string PseudoTerminal::GetSecondaryName() const {
- assert(m_primary_fd >= 0);
-#if HAVE_PTSNAME_R
- char buf[PATH_MAX];
- buf[0] = '\0';
- int r = ptsname_r(m_primary_fd, buf, sizeof(buf));
- (void)r;
- assert(r == 0);
- return buf;
-#else
+static std::string use_ptsname(int fd) {
static std::mutex mutex;
std::lock_guard guard(mutex);
- const char *r = ptsname(m_primary_fd);
+ const char *r = ptsname(fd);
assert(r != nullptr);
return r;
+}
+
+std::string PseudoTerminal::GetSecondaryName() const {
+ assert(m_primary_fd >= 0);
+#if HAVE_PTSNAME_R
+#if defined(__APPLE__)
+ if (__builtin_available(macos 10.13.4, iOS 11.3, tvOS 11.3, watchOS 4.4, *))
{
+#endif
+char buf[PATH_MAX];
+buf[0] = '\0';
+int r = ptsname_r(m_primary_fd, buf, sizeof(buf));
+(void)r;
+assert(r == 0);
+return buf;
+#if defined(__APPLE__)
+ } else {
+return use_ptsname(m_primary_fd);
+ }
+#endif
+#else
+ return use_ptsname(m_primary_fd);
#endif
}
Index: lldb/source/Host/common/PseudoTerminal.cpp
===
--- lldb/source/Host/common/PseudoTerminal.cpp
+++ lldb/source/Host/common/PseudoTerminal.cpp
@@ -22,6 +22,10 @@
#include "lldb/Host/PosixApi.h"
+#if defined(__APPLE__)
+#include
+#endif
+
#if defined(__ANDROID__)
int posix_openpt(int flags);
#endif
@@ -99,21 +103,33 @@
std::error_code(errno, std::generic_category()));
}
-std::string PseudoTerminal::GetSecondaryName() const {
- assert(m_primary_fd >= 0);
-#if HAVE_PTSNAME_R
- char buf[PATH_MAX];
- buf[0] = '\0';
- int r = ptsname_r(m_primary_fd, buf, sizeof(buf));
- (void)r;
- assert(r == 0);
- return buf;
-#else
+static std::string use_ptsname(int fd) {
static std::mutex mutex;
std::lock_guard guard(mutex);
- const char *r = ptsname(m_primary_fd);
+ const char *r = ptsname(fd);
assert(r != nullptr);
return r;
+}
+
+std::string PseudoTerminal::GetSecondaryName() const {
+ assert(m_primary_fd >= 0);
+#if HAVE_PTSNAME_R
+#if defined(__APPLE__)
+ if (__builtin_available(macos 10.13.4, iOS 11.3, tvOS 11.3, watchOS 4.4, *)) {
+#endif
+char buf[PATH_MAX];
+buf[0] = '\0';
+int r = ptsname_r(m_primary_fd, buf, sizeof(buf));
+(void)r;
+assert(r == 0);
+return buf;
+#if defined(__APPLE__)
+ } else {
+return use_ptsname(m_primary_fd);
+ }
+#endif
+#else
+ return use_ptsname(m_primary_fd);
#endif
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] b9a30b6 - [lldb] Update test_software_breakpoint_set_and_remove_work for AS
Author: Jonas Devlieghere Date: 2022-05-19T21:40:20-07:00 New Revision: b9a30b69d814971de5bd90a134b17b5954a8a2b4 URL: https://github.com/llvm/llvm-project/commit/b9a30b69d814971de5bd90a134b17b5954a8a2b4 DIFF: https://github.com/llvm/llvm-project/commit/b9a30b69d814971de5bd90a134b17b5954a8a2b4.diff LOG: [lldb] Update test_software_breakpoint_set_and_remove_work for AS On Apple Silicon the platform arch is arm64 rather than AArch64. Added: Modified: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py Removed: diff --git a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py index c6ff42a9a522f..8b3c8f0d914ba 100644 --- a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py +++ b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py @@ -812,7 +812,7 @@ def breakpoint_set_and_remove_work(self, want_hardware): target_arch = self.getArchitecture() # Set the breakpoint. -if (target_arch == "arm") or (target_arch == "aarch64"): +if target_arch in ["arm", "arm64", "aarch64"]: # TODO: Handle case when setting breakpoint in thumb code BREAKPOINT_KIND = 4 else: ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
