[Lldb-commits] [PATCH] D111779: [lldb] Make the thread_local g_global_boundary accessed from a single file
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Seems straight-forward enough, though I am wondering how much of this code is actually necessary given the deprecation/repurposing of reproducers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111779/new/ https://reviews.llvm.org/D111779 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D111355: [lldb] Add serial:// protocol for connecting to serial port [WIP/PoC]
labath added a comment. This looks reasonable to me. In case the url does not specify port parameters, it might be better to set them to known sane defaults instead of leaving them unchanged. That way lldb and lldb-server could "just connect". However, I don't know much about serial ports so I'm leaving that up to you. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111355/new/ https://reviews.llvm.org/D111355 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D111779: [lldb] Make the thread_local g_global_boundary accessed from a single file
mstorsjo created this revision.
mstorsjo added reviewers: labath, teemperor, JDevlieghere.
mstorsjo requested review of this revision.
Herald added a project: LLDB.
This makes the compiler generated code for accessing the thread local
variable much simpler (no need for wrapper functions and weak pointers
to potential init functions), and can avoid toolchain bugs regarding how
to access TLS variables.
In particular, this fixes LLDB when built with current GCC/binutils for
MinGW, see https://github.com/msys2/MINGW-packages/issues/8868.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D111779
Files:
lldb/include/lldb/Utility/ReproducerInstrumentation.h
lldb/source/Utility/ReproducerInstrumentation.cpp
Index: lldb/source/Utility/ReproducerInstrumentation.cpp
===
--- lldb/source/Utility/ReproducerInstrumentation.cpp
+++ lldb/source/Utility/ReproducerInstrumentation.cpp
@@ -16,6 +16,9 @@
using namespace lldb_private;
using namespace lldb_private::repro;
+// Whether we're currently across the API boundary.
+static thread_local bool g_global_boundary = false;
+
void *IndexToObject::GetObjectForIndexImpl(unsigned idx) {
return m_mapping.lookup(idx);
}
@@ -227,6 +230,13 @@
return m_sequence;
}
+void Recorder::PrivateThread() { g_global_boundary = true; }
+
+void Recorder::UpdateBoundary() {
+ if (m_local_boundary)
+g_global_boundary = false;
+}
+
void InstrumentationData::Initialize(Serializer &serializer,
Registry ®istry) {
InstanceImpl().emplace(serializer, registry);
@@ -248,6 +258,5 @@
return g_instrumentation_data;
}
-thread_local bool lldb_private::repro::Recorder::g_global_boundary = false;
std::atomic lldb_private::repro::Recorder::g_sequence;
std::mutex lldb_private::repro::Recorder::g_mutex;
Index: lldb/include/lldb/Utility/ReproducerInstrumentation.h
===
--- lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -868,17 +868,14 @@
/// Mark the current thread as a private thread and pretend that everything
/// on this thread is behind happening behind the API boundary.
- static void PrivateThread() { g_global_boundary = true; }
+ static void PrivateThread();
private:
static unsigned GetNextSequenceNumber() { return g_sequence++; }
unsigned GetSequenceNumber() const;
template friend struct replay;
- void UpdateBoundary() {
-if (m_local_boundary)
- g_global_boundary = false;
- }
+ void UpdateBoundary();
#ifdef LLDB_REPRO_INSTR_TRACE
void Log(unsigned id) {
@@ -902,9 +899,6 @@
/// The sequence number for this pair of function and result.
unsigned m_sequence;
- /// Whether we're currently across the API boundary.
- static thread_local bool g_global_boundary;
-
/// Global mutex to protect concurrent access.
static std::mutex g_mutex;
Index: lldb/source/Utility/ReproducerInstrumentation.cpp
===
--- lldb/source/Utility/ReproducerInstrumentation.cpp
+++ lldb/source/Utility/ReproducerInstrumentation.cpp
@@ -16,6 +16,9 @@
using namespace lldb_private;
using namespace lldb_private::repro;
+// Whether we're currently across the API boundary.
+static thread_local bool g_global_boundary = false;
+
void *IndexToObject::GetObjectForIndexImpl(unsigned idx) {
return m_mapping.lookup(idx);
}
@@ -227,6 +230,13 @@
return m_sequence;
}
+void Recorder::PrivateThread() { g_global_boundary = true; }
+
+void Recorder::UpdateBoundary() {
+ if (m_local_boundary)
+g_global_boundary = false;
+}
+
void InstrumentationData::Initialize(Serializer &serializer,
Registry ®istry) {
InstanceImpl().emplace(serializer, registry);
@@ -248,6 +258,5 @@
return g_instrumentation_data;
}
-thread_local bool lldb_private::repro::Recorder::g_global_boundary = false;
std::atomic lldb_private::repro::Recorder::g_sequence;
std::mutex lldb_private::repro::Recorder::g_mutex;
Index: lldb/include/lldb/Utility/ReproducerInstrumentation.h
===
--- lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -868,17 +868,14 @@
/// Mark the current thread as a private thread and pretend that everything
/// on this thread is behind happening behind the API boundary.
- static void PrivateThread() { g_global_boundary = true; }
+ static void PrivateThread();
private:
static unsigned GetNextSequenceNumber() { return g_sequence++; }
unsigned GetSequenceNumber() const;
template friend struct replay;
- void UpdateBoundary() {
-if (m_local_boundary)
- g_global_boundary = false;
- }
+ void UpdateBoundary();
#ifdef LLDB_REPRO_INSTR_TRACE
void Log(
[Lldb-commits] [PATCH] D108831: [lldb] [ABI/X86] Add pseudo-registers if missing
labath added a comment. We don't really want to support adding an arbitrary subset of sub-registers, do we? I am thinking if this could be made simpler if it was all-or-nothing. Like, during the initial pass you could check whether the list contains _any_ subregister, and abort if it does. Then the subsequent pass could assume that all subregisters need to be added... Comment at: lldb/source/Plugins/ABI/X86/ABIX86.h:23 + std::vector ®s, + bool is64bit); + I think you should be able to get this from the ArchSpec of the process. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108831/new/ https://reviews.llvm.org/D108831 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D94997: [lldb][lldb-vscode] Updated implementation of 'launch' and 'attach' requests to not create auxiliary target in case "launchCommands" and "attachCommands" are provided.
wallace added inline comments. Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:627-628 + "incompatible with 'attachCommands'.\n", arg.str().c_str()); + } +} +// Run any pre run LLDB commands the user specified in the launch.json clayborg wrote: > We can't print to stderr or stdout since this is where the VS code DAP > packets get delivered. > > We have two options here IMHO: > - deliver the warning/error stirng to the debugger console > - return an error with this string as the reason and fail the attach as long > as the error string get displayed to the user in the IDE > > We can deliver this to the "Debugger Console" using: > ``` > std::string str; > llvm::raw_string_ostream strm(str); > strm << ...; > g_vsc.SendOutput(OutputType::Console, strm.str()); > ``` > > do as Greg says and besides that terminate the debug session. This might be an indication of an erroneous configuration Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1696-1697 + "incompatible with 'launchCommands'.\n", arg.str().c_str()); + } +} +g_vsc.RunPreRunCommands(); clayborg wrote: > use g_vsc.SendOutput(OutputType::Console, ...) as mentioned above or return > an error. We will discuss the merits of message vs error in this comments. > same here, just terminate the session CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94997/new/ https://reviews.llvm.org/D94997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.
wallace added inline comments.
Comment at: lldb/include/lldb/Target/Process.h:241
- void BumpStopID() {
-m_stop_id++;
+ uint32_t BumpStopID() {
+const uint32_t prev_stop_id = m_stop_id++;
add a comment saying that this returns the stop id with the value before the
bump, otherwise someone might think that this returns the modified value
Comment at: lldb/include/lldb/Target/Statistics.h:23
+struct SuccessFailStats {
+ void Notify(bool success);
+
JDevlieghere wrote:
> I'd use an enum class with `Success` and `Failure` here instead of a bool
> that requires you to know what the argument is named to know if true means
> success of failure.
Or maybe just NotifySuccess(bool) to keep callers simple
Comment at: lldb/include/lldb/Target/Target.h:1466-1468
+ /// \param [in] modules
+ /// If true, include an array of metrics for each module loaded in the
+ /// target.
this param is not present
Comment at: lldb/include/lldb/Utility/ElapsedTime.h:22
+///
+/// Objects that need to measure elapsed times should have a variable with of
+/// type "ElapsedTime::Duration m_time_xxx;" which can then be used in the
Comment at: lldb/include/lldb/Utility/ElapsedTime.h:29
+///
+/// This class will update the m_time_xxx variable with the elapsed time when
+/// the object goes out of scope. The "m_time_xxx" variable will be incremented
Comment at: lldb/include/lldb/Utility/ElapsedTime.h:41
+ Timepoint m_start_time;
+ /// The elapsed time in seconds to update when this object goes out of scope.
+ ElapsedTime::Duration &m_elapsed_time;
Comment at: lldb/include/lldb/lldb-forward.h:121
class MemoryRegionInfos;
+struct StatsDumpOptions;
class Module;
move this to line ~277
Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:761
+def getShlibBuildArtifact(self, DYLIB_NAME):
+"""Return absolute path to a shared library artifact given the library
is this actually used somewhere?
Comment at: lldb/source/API/SBTarget.cpp:216-220
+ std::string json_text;
+ llvm::raw_string_ostream stream(json_text);
+ llvm::json::Value json = target_sp->ReportStatistics();
+ stream << json;
+ data.m_impl_up->SetObjectSP(StructuredData::ParseJSON(stream.str()));
a simpler version
Comment at: lldb/source/Commands/CommandObjectExpression.cpp:662-668
// Increment statistics to record this expression evaluation success.
-target.IncrementStats(StatisticKind::ExpressionSuccessful);
+target.GetStatistics().NotifyExprEval(true);
return true;
}
// Increment statistics to record this expression evaluation failure.
+ target.GetStatistics().NotifyExprEval(false);
actually this is not sufficient, we need to log as well in the SBAPI side,
which is what lldb-vscode uses to evaluate expressions. So either move it to
deeper point in the call chain or report this event from other locations
Comment at: lldb/source/Commands/CommandObjectFrame.cpp:711
bool res = result.Succeeded();
-Target &target = GetSelectedOrDummyTarget();
-if (res)
- target.IncrementStats(StatisticKind::FrameVarSuccess);
-else
- target.IncrementStats(StatisticKind::FrameVarFailure);
+GetSelectedOrDummyTarget().GetStatistics().NotifyFrameVar(res);
return res;
same here
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111686/new/
https://reviews.llvm.org/D111686
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.
JDevlieghere added a comment.
I have a few comments but I think this is going in a direction that will work
for both of us. Once this lands I can sign up for the work to untie the
statistics form the target.
Comment at: lldb/include/lldb/Target/Statistics.h:23
+struct SuccessFailStats {
+ void Notify(bool success);
+
I'd use an enum class with `Success` and `Failure` here instead of a bool that
requires you to know what the argument is named to know if true means success
of failure.
Comment at: lldb/include/lldb/Target/Statistics.h:28
+ uint32_t successes = 0;
+ uint32_t failures = 0;
+};
I would add a member for the name so that `ToJSON` can generate the whole JSON
object. I imagine something like
```
SuccessFailStats s("frameVariable");
s.Notify(SuccessFailStats::Success);
s.ToJSON()
```
which generates
```
"frameVariable": {
"failures": 0,
"successes": 1
},
```
Comment at: lldb/include/lldb/Target/Statistics.h:31
+
+class TargetStats {
+public:
The direction I outlined in the other patch is to move away from tying the
statistics to the target. I think the class itself is fine, as long as we agree
that the target stats would be held by an object spanning all debuggers.
Comment at: lldb/include/lldb/Target/Statistics.h:35
+
+ void SetLaunchOrAttachTime();
+ void SetFirstPrivateStopTime();
Why don't we have these return an `ElapsedTime`. Then you can do something like
```
ElapsedTime elapsed_time = m_stats.GetCreateTime();
```
RVO should ensure there are no copies, but you can always delete the copy ctor
to make sure. Maybe we should call it ScopedTime or something to make it more
obvious that this is a RAII object?
Comment at: lldb/include/lldb/Target/Statistics.h:51
+ // and "statistics disable".
+ bool m_collecting_stats = false;
+ ElapsedTime::Duration create_time{0.0};
In line with my earlier comment about not tying the stats tot he target, this
would be controlled by the global (singleton) statistic class.
Comment at: lldb/include/lldb/Target/Statistics.h:53
+ ElapsedTime::Duration create_time{0.0};
+ llvm::Optional launch_or_attach_time;
+ llvm::Optional
first_private_stop_time;
I would abstract this behind a `TimeStats` or `TimeStats`, similar to
SuccessFailStats, so that every type of statistic has its own type.
Comment at: lldb/include/lldb/Utility/ElapsedTime.h:34
+/// breakpoint each time a new shared library is loaded.
+class ElapsedTime {
+public:
This seems simple enough and specific enough to the statistics that it can be
part of that header?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111686/new/
https://reviews.llvm.org/D111686
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D94997: [lldb][lldb-vscode] Updated implementation of 'launch' and 'attach' requests to not create auxiliary target in case "launchCommands" and "attachCommands" are provided.
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
I am fine overall with this change because of goofy things that can happen when
we always create a target. We might think about returning an error if the user
specifies "launchCommands" or "attachCommands" if the user also includes any of
the arguments that will be ignored instead of printing to the console, see
inlined comments. I would vote for returning an error as long the the full
error string is displayed to the user and the entire string can be read and
isn't obfuscated in the launch/attach error dialog. I am not set on this and
would be interested to hear what others thing of the error vs showing something
in the debugger console
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:627-628
+ "incompatible with 'attachCommands'.\n", arg.str().c_str());
+ }
+}
+// Run any pre run LLDB commands the user specified in the launch.json
We can't print to stderr or stdout since this is where the VS code DAP packets
get delivered.
We have two options here IMHO:
- deliver the warning/error stirng to the debugger console
- return an error with this string as the reason and fail the attach as long as
the error string get displayed to the user in the IDE
We can deliver this to the "Debugger Console" using:
```
std::string str;
llvm::raw_string_ostream strm(str);
strm << ...;
g_vsc.SendOutput(OutputType::Console, strm.str());
```
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1696-1697
+ "incompatible with 'launchCommands'.\n", arg.str().c_str());
+ }
+}
+g_vsc.RunPreRunCommands();
use g_vsc.SendOutput(OutputType::Console, ...) as mentioned above or return an
error. We will discuss the merits of message vs error in this comments.
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1558
+ if (!launchCommands.empty()) {
+// if "launchCommands" are provided, then they are expected to make the
+// launch happen for launch requests and they replace the normal logic that
aadsm wrote:
> clayborg wrote:
> > We need to check if any arguments are set in the launch config that
> > "launchCommands" will ignore and return an error if any of them are set. Or
> > we need to emit an error or warning to the debug console so the users can
> > know that these settings are now ignored because "launchCommands" will
> > cause them to be.
> I vote for a warning here otherwise it might break people's current setups,
> assuming the error is an indication that the launch won't happen, but we
> should totally do it.
I kind of disagree after thinking about this some more. Right now we have many
things that can be specified in the launch config that will just get ignored
and if the user doesn't check the debugger console, they won't know. It is
probably ok to have the launch fail as long as the error string shows up and is
readable to the user and is complete enough for the user to read. Thoughts?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94997/new/
https://reviews.llvm.org/D94997
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D111659: [lldb] Skip several lldb tests that are flaky on Windows
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG58917054c298: [lldb] Skip several lldb tests that are flaky
on Windows (authored by stella.stamenova).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111659/new/
https://reviews.llvm.org/D111659
Files:
lldb/test/API/commands/process/attach/TestProcessAttach.py
lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py
lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
@@ -13,6 +13,7 @@
mydir = TestBase.compute_mydir(__file__)
+@skipIfWindows # This test is flaky on Windows
def test_attach_with_vAttachWait(self):
exe = '%s_%d' % (self.testMethodName, os.getpid())
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py
@@ -13,6 +13,7 @@
mydir = TestBase.compute_mydir(__file__)
+@skipIfWindows # This test is flaky on Windows
def test_launch_before_attach_with_vAttachOrWait(self):
exe = '%s_%d' % (self.testMethodName, os.getpid())
self.build(dictionary={'EXE': exe})
@@ -57,6 +58,7 @@
reported_pid = int(pid_text, base=16)
self.assertEqual(reported_pid, inferior.pid)
+@skipIfWindows # This test is flaky on Windows
def test_launch_after_attach_with_vAttachOrWait(self):
exe = '%s_%d' % (self.testMethodName, os.getpid())
self.build(dictionary={'EXE': exe})
Index:
lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
===
---
lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
+++
lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
@@ -16,6 +16,7 @@
NO_DEBUG_INFO_TESTCASE = True
@skipIfRemote
+@skipIfWindows # This test is flaky on Windows
def test_target_auto_install_main_executable(self):
if lldbgdbserverutils.get_lldb_server_exe() is None:
self.skipTest("lldb-server not found")
Index: lldb/test/API/commands/process/attach/TestProcessAttach.py
===
--- lldb/test/API/commands/process/attach/TestProcessAttach.py
+++ lldb/test/API/commands/process/attach/TestProcessAttach.py
@@ -101,6 +101,7 @@
process = target.GetProcess()
self.assertTrue(process, PROCESS_IS_VALID)
+@skipIfWindows # This test is flaky on Windows
@expectedFailureNetBSD
def test_attach_to_process_by_id_correct_executable_offset(self):
"""
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
@@ -13,6 +13,7 @@
mydir = TestBase.compute_mydir(__file__)
+@skipIfWindows # This test is flaky on Windows
def test_attach_with_vAttachWait(self):
exe = '%s_%d' % (self.testMethodName, os.getpid())
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py
@@ -13,6 +13,7 @@
mydir = TestBase.compute_mydir(__file__)
+@skipIfWindows # This test is flaky on Windows
def test_launch_before_attach_with_vAttachOrWait(self):
exe = '%s_%d' % (self.testMethodName, os.getpid())
self.build(dictionary={'EXE': exe})
@@ -57,6 +58,7 @@
reported_pid = int(pid_text, base=16)
self.assertEqual(reported_pid, inferior.pid)
+@skipIfWindows # This test is flaky on Windows
def test_launch_after_attach_with_vAttachOrWait(self):
exe = '%s_%d' % (self.testMethodName, os.getpid())
self.build(dictionary={'EXE': exe})
Index: lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
===
--- lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
+++ lldb/test/API/commands/target/auto-install-main-executable/TestAutoInsta
[Lldb-commits] [lldb] 5891705 - [lldb] Skip several lldb tests that are flaky on Windows
Author: Stella Stamenova
Date: 2021-10-13T09:46:41-07:00
New Revision: 58917054c29878ff3462f73b32a3e5dfa64d83f9
URL:
https://github.com/llvm/llvm-project/commit/58917054c29878ff3462f73b32a3e5dfa64d83f9
DIFF:
https://github.com/llvm/llvm-project/commit/58917054c29878ff3462f73b32a3e5dfa64d83f9.diff
LOG: [lldb] Skip several lldb tests that are flaky on Windows
These tests fail every 10 or so runs on Windows causing both local failures as
well as buildbot failures.
Differential Revision: https://reviews.llvm.org/D111659
Added:
Modified:
lldb/test/API/commands/process/attach/TestProcessAttach.py
lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py
lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
Removed:
diff --git a/lldb/test/API/commands/process/attach/TestProcessAttach.py
b/lldb/test/API/commands/process/attach/TestProcessAttach.py
index 265e9e1a5687d..b2d6878b13c0f 100644
--- a/lldb/test/API/commands/process/attach/TestProcessAttach.py
+++ b/lldb/test/API/commands/process/attach/TestProcessAttach.py
@@ -101,6 +101,7 @@ def test_attach_to_process_by_name(self):
process = target.GetProcess()
self.assertTrue(process, PROCESS_IS_VALID)
+@skipIfWindows # This test is flaky on Windows
@expectedFailureNetBSD
def test_attach_to_process_by_id_correct_executable_offset(self):
"""
diff --git
a/lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
b/lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
index 9993768df06f3..2bc9a05c5e1b9 100644
---
a/lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
+++
b/lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
@@ -16,6 +16,7 @@ class TestAutoInstallMainExecutable(TestBase):
NO_DEBUG_INFO_TESTCASE = True
@skipIfRemote
+@skipIfWindows # This test is flaky on Windows
def test_target_auto_install_main_executable(self):
if lldbgdbserverutils.get_lldb_server_exe() is None:
self.skipTest("lldb-server not found")
diff --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py
b/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py
index dd71e5077d47c..ded3ff6f849b6 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py
@@ -13,6 +13,7 @@ class
TestGdbRemoteAttachOrWait(gdbremote_testcase.GdbRemoteTestCaseBase):
mydir = TestBase.compute_mydir(__file__)
+@skipIfWindows # This test is flaky on Windows
def test_launch_before_attach_with_vAttachOrWait(self):
exe = '%s_%d' % (self.testMethodName, os.getpid())
self.build(dictionary={'EXE': exe})
@@ -57,6 +58,7 @@ def test_launch_before_attach_with_vAttachOrWait(self):
reported_pid = int(pid_text, base=16)
self.assertEqual(reported_pid, inferior.pid)
+@skipIfWindows # This test is flaky on Windows
def test_launch_after_attach_with_vAttachOrWait(self):
exe = '%s_%d' % (self.testMethodName, os.getpid())
self.build(dictionary={'EXE': exe})
diff --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
b/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
index deb82ed5183b1..980bcb9c9d7c0 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
@@ -13,6 +13,7 @@ class
TestGdbRemoteAttachWait(gdbremote_testcase.GdbRemoteTestCaseBase):
mydir = TestBase.compute_mydir(__file__)
+@skipIfWindows # This test is flaky on Windows
def test_attach_with_vAttachWait(self):
exe = '%s_%d' % (self.testMethodName, os.getpid())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D111715: [WIP] [lldb] change name demangling to be consistent between windows and linx
mstorsjo added a reviewer: thakis. mstorsjo added a subscriber: thakis. mstorsjo added a comment. The demangler change looks fine to me (but a test would indeed be necessary), but I think @thakis is the one who's been most involved with the MS demangler. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111715/new/ https://reviews.llvm.org/D111715 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 4019699 - [lldb] Add a test for CRTP
Author: Raphael Isemann
Date: 2021-10-13T17:15:02+02:00
New Revision: 4019699fa5fd153586f02fd7f6b7cfc51a688bf2
URL:
https://github.com/llvm/llvm-project/commit/4019699fa5fd153586f02fd7f6b7cfc51a688bf2
DIFF:
https://github.com/llvm/llvm-project/commit/4019699fa5fd153586f02fd7f6b7cfc51a688bf2.diff
LOG: [lldb] Add a test for CRTP
Added:
lldb/test/API/lang/cpp/crtp/Makefile
lldb/test/API/lang/cpp/crtp/TestCppCRTP.py
lldb/test/API/lang/cpp/crtp/main.cpp
Modified:
Removed:
diff --git a/lldb/test/API/lang/cpp/crtp/Makefile
b/lldb/test/API/lang/cpp/crtp/Makefile
new file mode 100644
index 0..8b20bcb05
--- /dev/null
+++ b/lldb/test/API/lang/cpp/crtp/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/cpp/crtp/TestCppCRTP.py
b/lldb/test/API/lang/cpp/crtp/TestCppCRTP.py
new file mode 100644
index 0..f511ad75455a4
--- /dev/null
+++ b/lldb/test/API/lang/cpp/crtp/TestCppCRTP.py
@@ -0,0 +1,36 @@
+"""
+A test for the curiously recurring template pattern (or CRTP).
+
+Note that the derived class is referenced directly from the parent class in the
+test. If this fails then there is a good chance that LLDB tried to eagerly
+resolve the definition of the derived class while constructing the base class.
+"""
+
+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__)
+
+@no_debug_info_test
+def test(self):
+self.build()
+self.createTestTarget()
+
+# Try using the class in the expression evaluator.
+self.expect_expr(
+"derived",
+result_type="Derived",
+result_children=[
+ValueCheck(name="Base"),
+ValueCheck(name="member", value="0"),
+],
+)
+
+# Try accessing the members of the class and base class.
+self.expect_expr("derived.pointer", result_type="Derived *")
+self.expect_expr("derived.member", result_type="int", result_value="0")
diff --git a/lldb/test/API/lang/cpp/crtp/main.cpp
b/lldb/test/API/lang/cpp/crtp/main.cpp
new file mode 100644
index 0..ed33115b88a93
--- /dev/null
+++ b/lldb/test/API/lang/cpp/crtp/main.cpp
@@ -0,0 +1,17 @@
+template struct Base {
+ Base(T &t) : ref(t), pointer(&t) {}
+ // Try referencing `Derived` via
diff erent ways to potentially make LLDB
+ // pull in the definition (which would recurse back to this base class).
+ T &ref;
+ T *pointer;
+ T func() { return ref; }
+};
+
+struct Derived : Base {
+ Derived() : Base(*this) {}
+ int member = 0;
+};
+
+Derived derived;
+
+int main() { return derived.member; }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D111715: [WIP] [lldb] change name demangling to be consistent between windows and linx
teemperor added reviewers: mstorsjo, rnk. teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Herald added a subscriber: JDevlieghere. This looks in general good to me. You probably want to add a test for this (`llvm/test/Demangle/ms-options.test` seems like the right place). I added some folks to review the Demangler changes. The LLDB itself LGTM, but it seems like this patch is also clang-formatting some lines that are unrelated to this patch, so please just clang-format those files as a preparatory NFC commit (If you don't have commit access then @werat or me can also just do that quickly for you). FWIW, there is a git clang-format script that limits clang-format changes to just the lines you touched. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111715/new/ https://reviews.llvm.org/D111715 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D111634: [lldb] Print embedded nuls in char arrays (PR44649)
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at:
lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp:40
+//% self.expect_var_path("c.data", summary=r'"F\0O"')
//% self.runCmd("setting set escape-non-printables false")
//% self.expect_var_path('stdstring', summary='"Hello\t\tWorld\nI am
here\t\tto say hello\n"')
I think the whole 'empty lines break inline tests' thing has been fixed at some
point, so could you drop a new line between this and the printable thing when
you land?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111634/new/
https://reviews.llvm.org/D111634
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D111715: [WIP] [lldb] change name demangling to be consistent between windows and linx
lassefolger created this revision.
lassefolger added reviewers: teemperor, werat.
Herald added a subscriber: hiraditya.
lassefolger requested review of this revision.
Herald added projects: LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits.
When printing names in lldb on windows these names contain the full type
information while on linux only the name is contained.
This change introduces a flag in the Microsoft demangler to control if the type
information should be included.
For globals there is a second inconsistency which is not yet addressed by this
change. On linux globals (in global namespace) are prefixed with :: while on
windows they are not.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D111715
Files:
lldb/source/Core/Mangled.cpp
llvm/include/llvm/Demangle/Demangle.h
llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
llvm/lib/Demangle/MicrosoftDemangle.cpp
llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
Index: llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
===
--- llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
+++ llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
@@ -613,12 +613,12 @@
if (!(Flags & OF_NoMemberType) && IsStatic)
OS << "static ";
- if (Type) {
+ if (!(Flags & OF_NoVariableType) && Type) {
Type->outputPre(OS, Flags);
outputSpaceIfNecessary(OS);
}
Name->output(OS, Flags);
- if (Type)
+ if (!(Flags & OF_NoVariableType) && Type)
Type->outputPost(OS, Flags);
}
Index: llvm/lib/Demangle/MicrosoftDemangle.cpp
===
--- llvm/lib/Demangle/MicrosoftDemangle.cpp
+++ llvm/lib/Demangle/MicrosoftDemangle.cpp
@@ -2361,6 +2361,8 @@
OF = OutputFlags(OF | OF_NoReturnType);
if (Flags & MSDF_NoMemberType)
OF = OutputFlags(OF | OF_NoMemberType);
+ if (Flags & MSDF_NoVariableType)
+OF = OutputFlags(OF | OF_NoVariableType);
int InternalStatus = demangle_success;
if (D.Error)
Index: llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
===
--- llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
+++ llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
@@ -80,6 +80,7 @@
OF_NoAccessSpecifier = 4,
OF_NoMemberType = 8,
OF_NoReturnType = 16,
+ OF_NoVariableType = 32,
};
// Types
Index: llvm/include/llvm/Demangle/Demangle.h
===
--- llvm/include/llvm/Demangle/Demangle.h
+++ llvm/include/llvm/Demangle/Demangle.h
@@ -31,7 +31,6 @@
char *itaniumDemangle(const char *mangled_name, char *buf, size_t *n,
int *status);
-
enum MSDemangleFlags {
MSDF_None = 0,
MSDF_DumpBackrefs = 1 << 0,
@@ -39,6 +38,7 @@
MSDF_NoCallingConvention = 1 << 2,
MSDF_NoReturnType = 1 << 3,
MSDF_NoMemberType = 1 << 4,
+ MSDF_NoVariableType = 1 << 5,
};
/// Demangles the Microsoft symbol pointed at by mangled_name and returns it.
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -131,9 +131,9 @@
static char *GetMSVCDemangledStr(const char *M) {
char *demangled_cstr = llvm::microsoftDemangle(
M, nullptr, nullptr, nullptr, nullptr,
- llvm::MSDemangleFlags(llvm::MSDF_NoAccessSpecifier |
-llvm::MSDF_NoCallingConvention |
-llvm::MSDF_NoMemberType));
+ llvm::MSDemangleFlags(
+ llvm::MSDF_NoAccessSpecifier | llvm::MSDF_NoCallingConvention |
+ llvm::MSDF_NoMemberType | llvm::MSDF_NoVariableType));
if (Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_DEMANGLE)) {
if (demangled_cstr && demangled_cstr[0])
@@ -260,7 +260,8 @@
if (m_mangled && m_demangled.IsNull()) {
// Don't bother running anything that isn't mangled
const char *mangled_name = m_mangled.GetCString();
-ManglingScheme mangling_scheme = GetManglingScheme(m_mangled.GetStringRef());
+ManglingScheme mangling_scheme =
+GetManglingScheme(m_mangled.GetStringRef());
if (mangling_scheme != eManglingSchemeNone &&
!m_mangled.GetMangledCounterpart(m_demangled)) {
// We didn't already mangle this name, demangle it and if all goes well
@@ -296,8 +297,7 @@
return m_demangled;
}
-ConstString
-Mangled::GetDisplayDemangledName() const {
+ConstString Mangled::GetDisplayDemangledName() const {
return GetDemangledName();
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 0648b3c - [lldb][NFC] for-range loop when iterating over delayed_properties
Author: Raphael Isemann Date: 2021-10-13T15:27:23+02:00 New Revision: 0648b3c0265e74a6920ae356885d0c29a1f6a44e URL: https://github.com/llvm/llvm-project/commit/0648b3c0265e74a6920ae356885d0c29a1f6a44e DIFF: https://github.com/llvm/llvm-project/commit/0648b3c0265e74a6920ae356885d0c29a1f6a44e.diff LOG: [lldb][NFC] for-range loop when iterating over delayed_properties Added: Modified: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Removed: diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index bed3dcbe570de..0c6d80449affb 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2147,10 +2147,8 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die, return true; }); -for (DelayedPropertyList::iterator pi = delayed_properties.begin(), - pe = delayed_properties.end(); - pi != pe; ++pi) - pi->Finalize(); +for (DelayedAddObjCClassProperty &property : delayed_properties) + property.Finalize(); } } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 7103753 - [lldb][NFC] Split out DW_TAG_inheritance parsing into own function
Author: Raphael Isemann
Date: 2021-10-13T13:14:57+02:00
New Revision: 7103753733a83602199958fb189d24f62c7400e8
URL:
https://github.com/llvm/llvm-project/commit/7103753733a83602199958fb189d24f62c7400e8
DIFF:
https://github.com/llvm/llvm-project/commit/7103753733a83602199958fb189d24f62c7400e8.diff
LOG: [lldb][NFC] Split out DW_TAG_inheritance parsing into own function
Just moving that block inside DWARFASTParserClang::ParseChildMembers into
its own function. Also early-exiting instead of a large if when
num_attributes is 0.
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 4af1c29ecb79f..bed3dcbe570de 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1406,6 +1406,123 @@ TypeSP DWARFASTParserClang::ParsePointerToMemberType(
return nullptr;
}
+void DWARFASTParserClang::ParseInheritance(
+const DWARFDIE &die, const DWARFDIE &parent_die,
+const CompilerType class_clang_type, const AccessType
default_accessibility,
+const lldb::ModuleSP &module_sp,
+std::vector> &base_classes,
+ClangASTImporter::LayoutInfo &layout_info) {
+
+ TypeSystemClang *ast =
+
llvm::dyn_cast_or_null(class_clang_type.GetTypeSystem());
+ if (ast == nullptr)
+return;
+
+ // TODO: implement DW_TAG_inheritance type parsing.
+ DWARFAttributes attributes;
+ const size_t num_attributes = die.GetAttributes(attributes);
+ if (num_attributes == 0)
+return;
+
+ DWARFFormValue encoding_form;
+ AccessType accessibility = default_accessibility;
+ bool is_virtual = false;
+ bool is_base_of_class = true;
+ off_t member_byte_offset = 0;
+
+ for (uint32_t i = 0; i < num_attributes; ++i) {
+const dw_attr_t attr = attributes.AttributeAtIndex(i);
+DWARFFormValue form_value;
+if (attributes.ExtractFormValueAtIndex(i, form_value)) {
+ switch (attr) {
+ case DW_AT_type:
+encoding_form = form_value;
+break;
+ case DW_AT_data_member_location:
+if (form_value.BlockData()) {
+ Value initialValue(0);
+ Value memberOffset(0);
+ const DWARFDataExtractor &debug_info_data = die.GetData();
+ uint32_t block_length = form_value.Unsigned();
+ uint32_t block_offset =
+ form_value.BlockData() - debug_info_data.GetDataStart();
+ if (DWARFExpression::Evaluate(
+ nullptr, nullptr, module_sp,
+ DataExtractor(debug_info_data, block_offset, block_length),
+ die.GetCU(), eRegisterKindDWARF, &initialValue, nullptr,
+ memberOffset, nullptr)) {
+member_byte_offset = memberOffset.ResolveValue(nullptr).UInt();
+ }
+} else {
+ // With DWARF 3 and later, if the value is an integer constant,
+ // this form value is the offset in bytes from the beginning of
+ // the containing entity.
+ member_byte_offset = form_value.Unsigned();
+}
+break;
+
+ case DW_AT_accessibility:
+accessibility = DW_ACCESS_to_AccessType(form_value.Unsigned());
+break;
+
+ case DW_AT_virtuality:
+is_virtual = form_value.Boolean();
+break;
+
+ default:
+break;
+ }
+}
+ }
+
+ Type *base_class_type = die.ResolveTypeUID(encoding_form.Reference());
+ if (base_class_type == nullptr) {
+module_sp->ReportError("0x%8.8x: DW_TAG_inheritance failed to "
+ "resolve the base class at 0x%8.8x"
+ " from enclosing type 0x%8.8x. \nPlease file "
+ "a bug and attach the file at the start of "
+ "this error message",
+ die.GetOffset(),
+ encoding_form.Reference().GetOffset(),
+ parent_die.GetOffset());
+return;
+ }
+
+ CompilerType base_class_clang_type = base_class_type->GetFullCompilerType();
+ assert(base_class_clang_type);
+ if (TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type)) {
+ast->SetObjCSuperClass(class_clang_type, base_class_clang_type);
+return;
+ }
+ std::unique_ptr result =
+ ast->CreateBaseClassSpecifier(base_class_clang_type.GetOpaqueQualType(),
+accessibility, is_virtual,
+is_base_of_class);
+ if (!result)
+return;
+
+ base_classes.push_back(std::move(result));
+
+ if (is_virtual) {
+// Do not specify any offset for virtual inheritance. The DWARF
+// produced by clang doesn't give us a constant offset
