[Lldb-commits] [lldb] [lldb][test] Modernize asserts (PR #82503)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jordan Rupprecht (rupprecht) Changes This uses [teyit](https://pypi.org/project/teyit/) to modernize asserts, as recommended by the [unittest release notes](https://docs.python.org/3.12/whatsnew/3.12.html#id3). For example, `assertTrue(a == b)` is replaced with `assertEqual(a, b)`. This produces better error messages, e.g. `error: unexpectedly found 1 and 2 to be different` instead of `error: False`. --- Patch is 112.26 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82503.diff 99 Files Affected: - (modified) lldb/test/API/commands/expression/call-restarts/TestCallThatRestarts.py (+1-1) - (modified) lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py (+2-2) - (modified) lldb/test/API/commands/expression/completion-crash-invalid-iterator/TestInvalidIteratorCompletionCrash.py (+1-1) - (modified) lldb/test/API/commands/expression/fixits/TestFixIts.py (+4-4) - (modified) lldb/test/API/commands/expression/test/TestExprs.py (+1-1) - (modified) lldb/test/API/commands/expression/unwind_expression/TestUnwindExpression.py (+3-2) - (modified) lldb/test/API/commands/register/register/aarch64_sme_z_registers/save_restore/TestSMEZRegistersSaveRestore.py (+1-1) - (modified) lldb/test/API/commands/register/register/aarch64_sme_z_registers/za_dynamic_resize/TestZAThreadedDynamic.py (+1-1) - (modified) lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py (+1-1) - (modified) lldb/test/API/commands/session/save/TestSessionSave.py (+1-1) - (modified) lldb/test/API/commands/statistics/basic/TestStats.py (+2-2) - (modified) lldb/test/API/commands/trace/TestTraceExport.py (+1-1) - (modified) lldb/test/API/commands/trace/TestTraceSave.py (+5-5) - (modified) lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py (+3-3) - (modified) lldb/test/API/functionalities/archives/TestBSDArchives.py (+6-4) - (modified) lldb/test/API/functionalities/asan/TestMemoryHistory.py (+3-3) - (modified) lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py (+12-8) - (modified) lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommandsFromPython.py (+3-1) - (modified) lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py (+2-2) - (modified) lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py (+6-6) - (modified) lldb/test/API/functionalities/breakpoint/objc/TestObjCBreakpoints.py (+9-6) - (modified) lldb/test/API/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py (+12-10) - (modified) lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py (+1-1) - (modified) lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py (+1-1) - (modified) lldb/test/API/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py (+14-10) - (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py (+5-5) - (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py (+5-5) - (modified) lldb/test/API/functionalities/gdb_remote_client/TestMSP430MSPDebug.py (+6-5) - (modified) lldb/test/API/functionalities/multidebugger_commands/TestMultipleDebuggersCommands.py (+6-4) - (modified) lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py (+2-2) - (modified) lldb/test/API/functionalities/return-value/TestReturnValue.py (+3-2) - (modified) lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py (+1-3) - (modified) lldb/test/API/functionalities/signal/TestSendSignal.py (+2-2) - (modified) lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py (+2-2) - (modified) lldb/test/API/functionalities/signal/handle-segv/TestHandleSegv.py (+2-2) - (modified) lldb/test/API/functionalities/signal/raise/TestRaise.py (+4-4) - (modified) lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py (+3-2) - (modified) lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py (+3-2) - (modified) lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py (+3-2) - (modified) lldb/test/API/functionalities/thread/break_after_join/TestBreakAfterJoin.py (+3-2) - (modified) lldb/test/API/functionalities/thread/create_during_step/TestCreateDuringStep.py (+6-4) - (modified) lldb/test/API/functionalities/thread/exit_during_break/TestExitDuringBreak.py (+3-2) - (modified) lldb/test/API/functionalities/thread/multi_break/TestMultipleBreakpoints.py (+3-2) - (modified) lldb/test/API/functionalities/thread/num_threa
[Lldb-commits] [lldb] [lldb][test] Modernize asserts (PR #82503)
https://github.com/bulbazord approved this pull request. I did a quick manual inspection of all of them, the transformation produced by Teyit looks correct to me. I did notice a few places where we were doing convoluted checks (I left a comment on one) but those should be addressed in follow-ups I think. https://github.com/llvm/llvm-project/pull/82503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Modernize asserts (PR #82503)
https://github.com/bulbazord edited https://github.com/llvm/llvm-project/pull/82503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Modernize asserts (PR #82503)
@@ -456,8 +457,9 @@ def queues_with_libBacktraceRecording(self): "doing_the_work_2", "queue 2's pending item #0 should be doing_the_work_2", ) -self.assertTrue( -queue_performer_2.GetPendingItemAtIndex().IsValid() == False, +self.assertEqual( +queue_performer_2.GetPendingItemAtIndex().IsValid(), bulbazord wrote: Might be useful to do an additional pass over these later. I think we could use `assertFalse` here, for example. https://github.com/llvm/llvm-project/pull/82503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Don't rely on ScriptInterpreterPythonImpl::Initialize in the unit tests (PR #82096)
DavidSpickett wrote: This change has caused a failing test on Linux: https://lab.llvm.org/buildbot/#/builders/68/builds/69157 That's not the first build, because the bot was red for ages beforehand and I'm not going to check them all. I've bisected it on arm64 instead to confirm it and get a better traceback: ``` TEST 'lldb-unit :: ScriptInterpreter/Python/./ScriptInterpreterPythonTests/32/39' FAILED Script(shard): -- GTEST_OUTPUT=json:/home/david.spickett/build-llvm-aarch64/tools/lldb/unittests/ScriptInterpreter/Python/./ScriptInterpreterPythonTests-lldb-unit-831825-32-39.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=39 GTEST_SHARD_INDEX=32 /home/david.spickett/build-llvm-aarch64/tools/lldb/unittests/ScriptInterpreter/Python/./ScriptInterpreterPythonTests -- Note: This is test shard 33 of 39. [==] Running 1 test from 1 test suite. [--] Global test environment set-up. [--] 1 test from PythonDataObjectsTest [ RUN ] PythonDataObjectsTest.TestPythonFile /usr/lib/gcc/aarch64-linux-gnu/13/../../../../include/c++/13/optional:477: _Tp &std::_Optional_base_impl>::_M_get() [_Tp = lldb_private::FileSystem, _Dp = std::_Optional_base]: Assertion 'this->_M_is_engaged()' failed. #0 0xc51c9630 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/david.spickett/build-llvm-aarch64/tools/lldb/unittests/ScriptInterpreter/Python/./ScriptInterpreterPythonTests+0xaf630) #1 0xc51c792c llvm::sys::RunSignalHandlers() (/home/david.spickett/build-llvm-aarch64/tools/lldb/unittests/ScriptInterpreter/Python/./ScriptInterpreterPythonTests+0xad92c) #2 0xc51ca088 SignalHandler(int) (/home/david.spickett/build-llvm-aarch64/tools/lldb/unittests/ScriptInterpreter/Python/./ScriptInterpreterPythonTests+0xb0088) #3 0xac90c5c0 (linux-vdso.so.1+0x5c0) #4 0xabba9d78 raise /build/glibc-RIFKjK/glibc-2.31/signal/../sysdeps/unix/sysv/linux/raise.c:51:1 #5 0xabb96aac abort /build/glibc-RIFKjK/glibc-2.31/stdlib/abort.c:81:7 #6 0xabde95d8 std::__glibcxx_assert_fail(char const*, int, char const*, char const*) (/lib/aarch64-linux-gnu/libstdc++.so.6+0xd75d8) #7 0xc51fa924 lldb_private::FileSystem::Instance() (/home/david.spickett/build-llvm-aarch64/tools/lldb/unittests/ScriptInterpreter/Python/./ScriptInterpreterPythonTests+0xe0924) #8 0xc5180b60 PythonDataObjectsTest_TestPythonFile_Test::TestBody() (/home/david.spickett/build-llvm-aarch64/tools/lldb/unittests/ScriptInterpreter/Python/./ScriptInterpreterPythonTests+0x66b60) ``` Seems like this Initialize had some side effects beyond Python itself? https://github.com/llvm/llvm-project/pull/82096 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Don't rely on ScriptInterpreterPythonImpl::Initialize in the unit tests (PR #82096)
@@ -9,43 +9,27 @@
#include "gtest/gtest.h"
#include "Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h"
-#include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h"
-#include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h"
#include "Plugins/ScriptInterpreter/Python/lldb-python.h"
-#include "lldb/Host/FileSystem.h"
-#include "lldb/Host/HostInfo.h"
-
#include "PythonTestSuite.h"
#include
-using namespace lldb_private;
-class TestScriptInterpreterPython : public ScriptInterpreterPythonImpl {
-public:
- using ScriptInterpreterPythonImpl::Initialize;
-};
-
void PythonTestSuite::SetUp() {
- FileSystem::Initialize();
DavidSpickett wrote:
This is perhaps the issue?
https://github.com/llvm/llvm-project/pull/82096
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Standardize command option parsing error messages (PR #82273)
https://github.com/bulbazord updated
https://github.com/llvm/llvm-project/pull/82273
>From 790810f9318c7947fe2edd187f60425a85c949b5 Mon Sep 17 00:00:00 2001
From: Alex Langford
Date: Thu, 15 Feb 2024 17:39:42 -0800
Subject: [PATCH 1/2] [lldb] Standardize command option parsing error messages
I have been looking to simplify parsing logic and improve the interfaces
so that they are both easier to use and harder to abuse. To be specific,
I am referring to functions such as `OptionArgParser::ToBoolean`: I
would like to go from its current interface to something more like
`llvm::Error ToBoolean(llvm::StringRef option_arg)`.
Through working on that, I encountered 2 inconveniences:
1. Option parsing code is not uniform. Every function writes a slightly
different error message, so incorporating an error message from the
`ToBoolean` implementation is going to be laborious as I figure out
what exactly needs to change or stay the same.
2. Changing the interface of `ToBoolean` would require a global atomic
change across all of the Command code. This would be quite
frustrating to do because of the non-uniformity of our existing code.
To address these frustrations, I think it would be easiest to first
standardize the error reporting mechanism when parsing options in
commands. I do so by introducing `CreateOptionParsingError` which will
create an error message of the shape:
Invalid valud ('${option_arg}') for -${short_value} ('${long_value}'):
${additional_context}
Concretely, it would look something like this:
(lldb) breakpoint set -n main -G yay
error: Invalid value ('yay') for -G (auto-continue): Failed to parse as boolean
After this, updating the interfaces for parsing the values themselves
should become simpler. Because this can be adopted incrementally, this
should be able to done over the course of time instead of all at once as
a giant difficult-to-review change. I've changed exactly one function
where this function would be used as an illustration of what I am
proposing.
---
lldb/include/lldb/Interpreter/Options.h | 10 +
.../Commands/CommandObjectBreakpoint.cpp | 37 ++-
lldb/source/Interpreter/Options.cpp | 13 +++
lldb/unittests/Interpreter/CMakeLists.txt | 1 +
lldb/unittests/Interpreter/TestOptions.cpp| 29 +++
5 files changed, 73 insertions(+), 17 deletions(-)
create mode 100644 lldb/unittests/Interpreter/TestOptions.cpp
diff --git a/lldb/include/lldb/Interpreter/Options.h
b/lldb/include/lldb/Interpreter/Options.h
index bf74927cf99db8..3351fb517d4df3 100644
--- a/lldb/include/lldb/Interpreter/Options.h
+++ b/lldb/include/lldb/Interpreter/Options.h
@@ -336,6 +336,16 @@ class OptionGroupOptions : public Options {
bool m_did_finalize = false;
};
+llvm::Error CreateOptionParsingError(llvm::StringRef option_arg,
+ const char short_option,
+ llvm::StringRef long_option = {},
+ llvm::StringRef additional_context = {});
+
+static constexpr llvm::StringLiteral g_bool_parsing_error_message =
+"Failed to parse as boolean";
+static constexpr llvm::StringLiteral g_int_parsing_error_message =
+"Failed to parse as integer";
+
} // namespace lldb_private
#endif // LLDB_INTERPRETER_OPTIONS_H
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index 3fdf5cd3cd43d2..fc2217608a0bb9 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -64,6 +64,8 @@ class lldb_private::BreakpointOptionGroup : public
OptionGroup {
Status error;
const int short_option =
g_breakpoint_modify_options[option_idx].short_option;
+const char *long_option =
+g_breakpoint_modify_options[option_idx].long_option;
switch (short_option) {
case 'c':
@@ -84,18 +86,17 @@ class lldb_private::BreakpointOptionGroup : public
OptionGroup {
case 'G': {
bool value, success;
value = OptionArgParser::ToBoolean(option_arg, false, &success);
- if (success) {
+ if (success)
m_bp_opts.SetAutoContinue(value);
- } else
-error.SetErrorStringWithFormat(
-"invalid boolean value '%s' passed for -G option",
-option_arg.str().c_str());
+ else
+error = CreateOptionParsingError(option_arg, short_option, long_option,
+ g_bool_parsing_error_message);
} break;
case 'i': {
uint32_t ignore_count;
if (option_arg.getAsInteger(0, ignore_count))
-error.SetErrorStringWithFormat("invalid ignore count '%s'",
- option_arg.str().c_str());
+error = CreateOptionParsingError(option_arg, short_option, long_option,
+ g_int_parsing_error_message);
else
m_bp_o
[Lldb-commits] [lldb] [lldb] Standardize command option parsing error messages (PR #82273)
bulbazord wrote: I've added a doxygen comment to the new function I introduced. I plan on landing this later today if there are no objections. https://github.com/llvm/llvm-project/pull/82273 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Modernize asserts (PR #82503)
@@ -456,8 +457,9 @@ def queues_with_libBacktraceRecording(self): "doing_the_work_2", "queue 2's pending item #0 should be doing_the_work_2", ) -self.assertTrue( -queue_performer_2.GetPendingItemAtIndex().IsValid() == False, +self.assertEqual( +queue_performer_2.GetPendingItemAtIndex().IsValid(), rupprecht wrote: Good point -- I can check for this kind of case as a followup. https://github.com/llvm/llvm-project/pull/82503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Modernize asserts (PR #82503)
https://github.com/rupprecht closed https://github.com/llvm/llvm-project/pull/82503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Modernize assertEqual(value, bool) (PR #82526)
https://github.com/rupprecht created
https://github.com/llvm/llvm-project/pull/82526
Any time we see the pattern `assertEqual(value, bool)`, we can replace that
with `assert(value)`. Likewise for `assertNotEqual`.
Technically this relaxes the test a bit, as we may want to make sure `value` is
either `True` or `False`, and not something that implicitly converts to a bool.
For example, `assertEqual("foo", True)` will fail, but `assertTrue("foo")` will
not. In most cases, this distinction is not important.
There are two such places that this patch does **not** transform, since it
seems intentional that we want the result to be a bool:
*
https://github.com/llvm/llvm-project/blob/5daf2001a1e4d71ce1273a1e7e31cf6e6ac37c10/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py#L90
*
https://github.com/llvm/llvm-project/blob/5daf2001a1e4d71ce1273a1e7e31cf6e6ac37c10/lldb/test/API/commands/settings/TestSettings.py#L940
Followup to 9c2468821ec51defd09c246fea4a47886fff8c01. I patched `teyit` with a
`visit_assertEqual` node handler to generate this.
>From b2e8b0c35dc6ef95016ad04e9ddf8c55043fe2e8 Mon Sep 17 00:00:00 2001
From: Jordan Rupprecht
Date: Wed, 21 Feb 2024 11:45:36 -0800
Subject: [PATCH 1/2] [lldb][test] Modernize assertEqual(value, bool)
Any time we see the pattern `assertEqual(value, bool)`, we can replace that
with `assert(value)`. Likewise for `assertNotEqual`.
Technically this relaxes the test a bit, as we may want to make sure `value` is
either `True` or `False`, and not something that implicitly converts to a bool.
For example, `assertEqual("foo", True)` will fail, but `assertTrue("foo")` will
not. The cases here seem correct.
Followup to 9c2468821ec51defd09c246fea4a47886fff8c01. I patched `teyit` with a
`visit_assertEqual` node handler to generate this.
---
.../call-throws/TestCallThatThrows.py | 2 +-
.../expression/dont_allow_jit/TestAllowJIT.py | 8 ++--
.../API/commands/settings/TestSettings.py | 2 +-
.../commands/statistics/basic/TestStats.py| 34 ++---
lldb/test/API/commands/trace/TestTraceSave.py | 8 ++--
.../TestBadAddressBreakpoints.py | 2 +-
.../TestBreakpointCommand.py | 12 ++---
.../breakpoint_names/TestBreakpointNames.py | 20 +++-
.../TestJLink6Armv7RegisterDefinition.py | 2 +-
.../simple_exe/TestModuleCacheSimple.py | 13 ++---
.../stats_api/TestStatisticsAPI.py| 48 +--
.../backtrace_limit/TestBacktraceLimit.py | 2 +-
.../TestArmMachoCorefileRegctx.py | 4 +-
.../addrable-bits/TestAddrableBitsCorefile.py | 2 +-
.../TestFirmwareCorefiles.py | 14 +++---
.../kern-ver-str/TestKernVerStrLCNOTE.py | 2 +-
.../TestMultipleBinaryCorefile.py | 2 +-
lldb/test/API/macosx/queues/TestQueues.py | 3 +-
.../safe-to-func-call/TestSafeFuncCalls.py| 3 +-
.../TestRunCommandInterpreterAPI.py | 28 +--
20 files changed, 96 insertions(+), 115 deletions(-)
diff --git
a/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py
b/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py
index 2868ec5ffdbdf8..b8cc87c93ba61d 100644
--- a/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py
+++ b/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py
@@ -46,7 +46,7 @@ def call_function(self):
value = frame.EvaluateExpression("[my_class callMeIThrow]", options)
self.assertTrue(value.IsValid())
-self.assertEqual(value.GetError().Success(), False)
+self.assertFalse(value.GetError().Success())
self.check_after_call()
diff --git a/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
b/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
index 307d4521427dcf..eb812f1902e662 100644
--- a/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
+++ b/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
@@ -54,7 +54,7 @@ def expr_options_test(self):
# First make sure we can call the function with the default option set.
options = lldb.SBExpressionOptions()
# Check that the default is to allow JIT:
-self.assertEqual(options.GetAllowJIT(), True, "Default is true")
+self.assertTrue(options.GetAllowJIT(), "Default is true")
# Now use the options:
result = frame.EvaluateExpression("call_me(10)", options)
@@ -64,9 +64,7 @@ def expr_options_test(self):
# Now disallow JIT and make sure it fails:
options.SetAllowJIT(False)
# Check that we got the right value:
-self.assertEqual(
-options.GetAllowJIT(), False, "Got False after setting to False"
-)
+self.assertFalse(options.GetAllowJIT(), "Got False after setting to
False")
# Again use it and ensure we fail:
result = frame.EvaluateExpression("call_me(10)", options)
@@
[Lldb-commits] [lldb] [lldb][test] Modernize assertEqual(value, bool) (PR #82526)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jordan Rupprecht (rupprecht) Changes Any time we see the pattern `assertEqual(value, bool)`, we can replace that with `assert(value)`. Likewise for `assertNotEqual`. Technically this relaxes the test a bit, as we may want to make sure `value` is either `True` or `False`, and not something that implicitly converts to a bool. For example, `assertEqual("foo", True)` will fail, but `assertTrue("foo")` will not. In most cases, this distinction is not important. There are two such places that this patch does **not** transform, since it seems intentional that we want the result to be a bool: * https://github.com/llvm/llvm-project/blob/5daf2001a1e4d71ce1273a1e7e31cf6e6ac37c10/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py#L90 * https://github.com/llvm/llvm-project/blob/5daf2001a1e4d71ce1273a1e7e31cf6e6ac37c10/lldb/test/API/commands/settings/TestSettings.py#L940 Followup to 9c2468821ec51defd09c246fea4a47886fff8c01. I patched `teyit` with a `visit_assertEqual` node handler to generate this. --- Patch is 28.83 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82526.diff 19 Files Affected: - (modified) lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py (+1-1) - (modified) lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py (+3-5) - (modified) lldb/test/API/commands/statistics/basic/TestStats.py (+14-20) - (modified) lldb/test/API/commands/trace/TestTraceSave.py (+4-4) - (modified) lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py (+1-1) - (modified) lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py (+6-6) - (modified) lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py (+7-13) - (modified) lldb/test/API/functionalities/gdb_remote_client/TestJLink6Armv7RegisterDefinition.py (+1-1) - (modified) lldb/test/API/functionalities/module_cache/simple_exe/TestModuleCacheSimple.py (+5-8) - (modified) lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py (+24-24) - (modified) lldb/test/API/functionalities/thread/backtrace_limit/TestBacktraceLimit.py (+1-1) - (modified) lldb/test/API/macosx/arm-corefile-regctx/TestArmMachoCorefileRegctx.py (+2-2) - (modified) lldb/test/API/macosx/lc-note/addrable-bits/TestAddrableBitsCorefile.py (+1-1) - (modified) lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py (+7-7) - (modified) lldb/test/API/macosx/lc-note/kern-ver-str/TestKernVerStrLCNOTE.py (+1-1) - (modified) lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py (+1-1) - (modified) lldb/test/API/macosx/queues/TestQueues.py (+1-2) - (modified) lldb/test/API/macosx/safe-to-func-call/TestSafeFuncCalls.py (+1-2) - (modified) lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py (+14-14) ``diff diff --git a/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py b/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py index 2868ec5ffdbdf8..b8cc87c93ba61d 100644 --- a/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py +++ b/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py @@ -46,7 +46,7 @@ def call_function(self): value = frame.EvaluateExpression("[my_class callMeIThrow]", options) self.assertTrue(value.IsValid()) -self.assertEqual(value.GetError().Success(), False) +self.assertFalse(value.GetError().Success()) self.check_after_call() diff --git a/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py b/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py index 307d4521427dcf..eb812f1902e662 100644 --- a/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py +++ b/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py @@ -54,7 +54,7 @@ def expr_options_test(self): # First make sure we can call the function with the default option set. options = lldb.SBExpressionOptions() # Check that the default is to allow JIT: -self.assertEqual(options.GetAllowJIT(), True, "Default is true") +self.assertTrue(options.GetAllowJIT(), "Default is true") # Now use the options: result = frame.EvaluateExpression("call_me(10)", options) @@ -64,9 +64,7 @@ def expr_options_test(self): # Now disallow JIT and make sure it fails: options.SetAllowJIT(False) # Check that we got the right value: -self.assertEqual( -options.GetAllowJIT(), False, "Got False after setting to False" -) +self.assertFalse(options.GetAllowJIT(), "Got False after setting to False") # Again use it and ensure we fail: result = frame.EvaluateExpression("call_me(10)", options) @@ -79,7 +77,7 @@ def expr_options_te
[Lldb-commits] [lldb] [lldb][test] Modernize asserts (PR #82503)
@@ -456,8 +457,9 @@ def queues_with_libBacktraceRecording(self): "doing_the_work_2", "queue 2's pending item #0 should be doing_the_work_2", ) -self.assertTrue( -queue_performer_2.GetPendingItemAtIndex().IsValid() == False, +self.assertEqual( +queue_performer_2.GetPendingItemAtIndex().IsValid(), rupprecht wrote: https://github.com/llvm/llvm-project/pull/82526 includes this and more https://github.com/llvm/llvm-project/pull/82503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] c63e68b - Bump the minimum LLVM version for TestTypeList.py
Author: Shubham Sandeep Rastogi
Date: 2024-02-21T13:42:10-08:00
New Revision: c63e68ba5fb54b69521c4f010d1c5290856c6509
URL:
https://github.com/llvm/llvm-project/commit/c63e68ba5fb54b69521c4f010d1c5290856c6509
DIFF:
https://github.com/llvm/llvm-project/commit/c63e68ba5fb54b69521c4f010d1c5290856c6509.diff
LOG: Bump the minimum LLVM version for TestTypeList.py
Added:
Modified:
lldb/test/API/python_api/type/TestTypeList.py
Removed:
diff --git a/lldb/test/API/python_api/type/TestTypeList.py
b/lldb/test/API/python_api/type/TestTypeList.py
index e75affd6522115..eba5e17355c3f8 100644
--- a/lldb/test/API/python_api/type/TestTypeList.py
+++ b/lldb/test/API/python_api/type/TestTypeList.py
@@ -18,6 +18,7 @@ def setUp(self):
self.source = "main.cpp"
self.line = line_number(self.source, "// Break at this line")
+@skipIf(compiler="clang", compiler_version=["<", "17.0"])
def test(self):
"""Exercise SBType and SBTypeList API."""
d = {"EXE": self.exe_name}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Modernize assertEqual(value, bool) (PR #82526)
https://github.com/bulbazord approved this pull request. Thanks! https://github.com/llvm/llvm-project/pull/82526 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 1eeeab8 - [lldb][test] Modernize assertEqual(value, bool) (#82526)
Author: Jordan Rupprecht
Date: 2024-02-21T20:39:02-06:00
New Revision: 1eeeab82c6eb185f5139e633a59c2dbcb15616e4
URL:
https://github.com/llvm/llvm-project/commit/1eeeab82c6eb185f5139e633a59c2dbcb15616e4
DIFF:
https://github.com/llvm/llvm-project/commit/1eeeab82c6eb185f5139e633a59c2dbcb15616e4.diff
LOG: [lldb][test] Modernize assertEqual(value, bool) (#82526)
Any time we see the pattern `assertEqual(value, bool)`, we can replace
that with `assert(value)`. Likewise for `assertNotEqual`.
Technically this relaxes the test a bit, as we may want to make sure
`value` is either `True` or `False`, and not something that implicitly
converts to a bool. For example, `assertEqual("foo", True)` will fail,
but `assertTrue("foo")` will not. In most cases, this distinction is not
important.
There are two such places that this patch does **not** transform, since
it seems intentional that we want the result to be a bool:
*
https://github.com/llvm/llvm-project/blob/5daf2001a1e4d71ce1273a1e7e31cf6e6ac37c10/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py#L90
*
https://github.com/llvm/llvm-project/blob/5daf2001a1e4d71ce1273a1e7e31cf6e6ac37c10/lldb/test/API/commands/settings/TestSettings.py#L940
Followup to 9c2468821ec51defd09c246fea4a47886fff8c01. I patched `teyit`
with a `visit_assertEqual` node handler to generate this.
Added:
Modified:
lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py
lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
lldb/test/API/commands/statistics/basic/TestStats.py
lldb/test/API/commands/trace/TestTraceSave.py
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
lldb/test/API/functionalities/gdb_remote_client/TestJLink6Armv7RegisterDefinition.py
lldb/test/API/functionalities/module_cache/simple_exe/TestModuleCacheSimple.py
lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py
lldb/test/API/functionalities/thread/backtrace_limit/TestBacktraceLimit.py
lldb/test/API/macosx/arm-corefile-regctx/TestArmMachoCorefileRegctx.py
lldb/test/API/macosx/lc-note/addrable-bits/TestAddrableBitsCorefile.py
lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
lldb/test/API/macosx/lc-note/kern-ver-str/TestKernVerStrLCNOTE.py
lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py
lldb/test/API/macosx/queues/TestQueues.py
lldb/test/API/macosx/safe-to-func-call/TestSafeFuncCalls.py
lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py
Removed:
diff --git
a/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py
b/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py
index 2868ec5ffdbdf8..b8cc87c93ba61d 100644
--- a/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py
+++ b/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py
@@ -46,7 +46,7 @@ def call_function(self):
value = frame.EvaluateExpression("[my_class callMeIThrow]", options)
self.assertTrue(value.IsValid())
-self.assertEqual(value.GetError().Success(), False)
+self.assertFalse(value.GetError().Success())
self.check_after_call()
diff --git a/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
b/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
index 307d4521427dcf..eb812f1902e662 100644
--- a/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
+++ b/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
@@ -54,7 +54,7 @@ def expr_options_test(self):
# First make sure we can call the function with the default option set.
options = lldb.SBExpressionOptions()
# Check that the default is to allow JIT:
-self.assertEqual(options.GetAllowJIT(), True, "Default is true")
+self.assertTrue(options.GetAllowJIT(), "Default is true")
# Now use the options:
result = frame.EvaluateExpression("call_me(10)", options)
@@ -64,9 +64,7 @@ def expr_options_test(self):
# Now disallow JIT and make sure it fails:
options.SetAllowJIT(False)
# Check that we got the right value:
-self.assertEqual(
-options.GetAllowJIT(), False, "Got False after setting to False"
-)
+self.assertFalse(options.GetAllowJIT(), "Got False after setting to
False")
# Again use it and ensure we fail:
result = frame.EvaluateExpression("call_me(10)", options)
@@ -79,7 +77,7 @@ def expr_options_test(self):
# Finally set the allow JIT value back to true and make sure that
works:
options.SetAllowJIT(
[Lldb-commits] [lldb] [lldb][test] Modernize assertEqual(value, bool) (PR #82526)
https://github.com/rupprecht closed https://github.com/llvm/llvm-project/pull/82526 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 7e1432f - [lldb] Standardize command option parsing error messages (#82273)
Author: Alex Langford
Date: 2024-02-21T19:26:43-08:00
New Revision: 7e1432f1258e229a4fcc9c017937166f0578e1f8
URL:
https://github.com/llvm/llvm-project/commit/7e1432f1258e229a4fcc9c017937166f0578e1f8
DIFF:
https://github.com/llvm/llvm-project/commit/7e1432f1258e229a4fcc9c017937166f0578e1f8.diff
LOG: [lldb] Standardize command option parsing error messages (#82273)
I have been looking to simplify parsing logic and improve the interfaces
so that they are both easier to use and harder to abuse. To be specific,
I am referring to functions such as `OptionArgParser::ToBoolean`: I
would like to go from its current interface to something more like
`llvm::Error ToBoolean(llvm::StringRef option_arg)`.
Through working on that, I encountered 2 inconveniences:
1. Option parsing code is not uniform. Every function writes a slightly
different error message, so incorporating an error message from the
`ToBoolean` implementation is going to be laborious as I figure out what
exactly needs to change or stay the same.
2. Changing the interface of `ToBoolean` would require a global atomic
change across all of the Command code. This would be quite frustrating
to do because of the non-uniformity of our existing code.
To address these frustrations, I think it would be easiest to first
standardize the error reporting mechanism when parsing options in
commands. I do so by introducing `CreateOptionParsingError` which will
create an error message of the shape:
Invalid value ('${option_arg}') for -${short_value} ('${long_value}'):
${additional_context}
Concretely, it would look something like this:
(lldb) breakpoint set -n main -G yay
error: Invalid value ('yay') for -G (auto-continue): Failed to parse as
boolean
After this, updating the interfaces for parsing the values themselves
should become simpler. Because this can be adopted incrementally, this
should be able to done over the course of time instead of all at once as
a giant difficult-to-review change. I've changed exactly one function
where this function would be used as an illustration of what I am
proposing.
Added:
lldb/unittests/Interpreter/TestOptions.cpp
Modified:
lldb/include/lldb/Interpreter/Options.h
lldb/source/Commands/CommandObjectBreakpoint.cpp
lldb/source/Interpreter/Options.cpp
lldb/unittests/Interpreter/CMakeLists.txt
Removed:
diff --git a/lldb/include/lldb/Interpreter/Options.h
b/lldb/include/lldb/Interpreter/Options.h
index bf74927cf99db8..18a87e49deee5c 100644
--- a/lldb/include/lldb/Interpreter/Options.h
+++ b/lldb/include/lldb/Interpreter/Options.h
@@ -336,6 +336,39 @@ class OptionGroupOptions : public Options {
bool m_did_finalize = false;
};
+/// Creates an error that represents the failure to parse an command line
option
+/// argument. This creates an error containing all information needed to show
+/// the developer what went wrong when parsing their command. It is recommended
+/// to use this instead of writing an error by hand.
+///
+/// \param[in] option_arg
+/// The argument that was attempted to be parsed.
+///
+/// \param[in] short_option
+/// The short form of the option. For example, if the flag is -f, the short
+/// option is "f".
+///
+/// \param[in] long_option
+/// The long form of the option. This field is optional. If the flag is
+/// --force, then the long option is "force".
+///
+/// \param[in] additional_context
+/// This is extra context that will get included in the error. This field is
+/// optional.
+///
+/// \return
+/// An llvm::Error that contains a standardized format for what went wrong
+/// when parsing and why.
+llvm::Error CreateOptionParsingError(llvm::StringRef option_arg,
+ const char short_option,
+ llvm::StringRef long_option = {},
+ llvm::StringRef additional_context = {});
+
+static constexpr llvm::StringLiteral g_bool_parsing_error_message =
+"Failed to parse as boolean";
+static constexpr llvm::StringLiteral g_int_parsing_error_message =
+"Failed to parse as integer";
+
} // namespace lldb_private
#endif // LLDB_INTERPRETER_OPTIONS_H
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index 3fdf5cd3cd43d2..fc2217608a0bb9 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -64,6 +64,8 @@ class lldb_private::BreakpointOptionGroup : public
OptionGroup {
Status error;
const int short_option =
g_breakpoint_modify_options[option_idx].short_option;
+const char *long_option =
+g_breakpoint_modify_options[option_idx].long_option;
switch (short_option) {
case 'c':
@@ -84,18 +86,17 @@ class lldb_private::BreakpointOptionGroup : public
OptionGroup {
case 'G': {
bool value, succes
[Lldb-commits] [lldb] [lldb] Standardize command option parsing error messages (PR #82273)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/82273 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPack… (PR #82593)
https://github.com/jasonmolenda created
https://github.com/llvm/llvm-project/pull/82593
…etSupported
Pavel added an extension to lldb's gdb remote serial protocol that allows the
debug stub to append an error message (ascii hex encoded) after an error
response packet Exx. This was added in 2017 in https://reviews.llvm.org/D34945
. lldb sends the
QErrorStringInPacketSupported packet and then the remote stub may add these
error strings.
debugserver has added these strings to the vAttach family of packets to explain
why an attach failed, without waiting to see the QErrorStringInPacketSupported
packet to enable the mode.
debugserver also has a bad implementation of this for the qLaunchSuccess
packet, where "E" is sent followed by the literal characters of the error,
instead of Exx;, which lldb can have problems parsing.
This patch moves three utility functions earlier in RNBRemote.cpp, it accepts
the QErrorStringInPacketSupported packet and only appends the expanded error
messages when that has been sent (lldb always sends it at the beginning of a
connection), fixes the error string returned by qLaunchSuccess and changes the
vAttach error returns to only add the error string when the mode is enabled.
>From 0ba4e6402969028fa6152c366a56063a56acded1 Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Wed, 21 Feb 2024 22:48:36 -0800
Subject: [PATCH] [lldb] [debugserver] fix qLaunchSuccess error, add
QErrorStringInPacketSupported
Pavel added an extension to lldb's gdb remote serial protocol that
allows the debug stub to append an error message (ascii hex encoded)
after an error response packet Exx. This was added in 2017 in
https://reviews.llvm.org/D34945 . lldb sends the
QErrorStringInPacketSupported packet and then the remote stub may
add these error strings.
debugserver has added these strings to the vAttach family of packets
to explain why an attach failed, without waiting to see the
QErrorStringInPacketSupported packet to enable the mode.
debugserver also has a bad implementation of this for the qLaunchSuccess
packet, where "E" is sent followed by the literal characters of the
error, instead of Exx;, which lldb can have problems
parsing.
This patch moves three utility functions earlier in RNBRemote.cpp,
it accepts the QErrorStringInPacketSupported packet and only appends
the expanded error messages when that has been sent (lldb always sends
it at the beginning of a connection), fixes the error string returned
by qLaunchSuccess and changes the vAttach error returns to only add
the error string when the mode is enabled.
---
lldb/tools/debugserver/source/RNBRemote.cpp | 157
lldb/tools/debugserver/source/RNBRemote.h | 5 +
2 files changed, 103 insertions(+), 59 deletions(-)
diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp
b/lldb/tools/debugserver/source/RNBRemote.cpp
index feea4c914ec536..8338e4d0032870 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -143,6 +143,39 @@ uint64_t decode_uint64(const char *p, int base, char **end
= nullptr,
return addr;
}
+void append_hex_value(std::ostream &ostrm, const void *buf, size_t buf_size,
+ bool swap) {
+ int i;
+ const uint8_t *p = (const uint8_t *)buf;
+ if (swap) {
+for (i = static_cast(buf_size) - 1; i >= 0; i--)
+ ostrm << RAWHEX8(p[i]);
+ } else {
+for (size_t i = 0; i < buf_size; i++)
+ ostrm << RAWHEX8(p[i]);
+ }
+}
+
+std::string cstring_to_asciihex_string(const char *str) {
+ std::string hex_str;
+ hex_str.reserve(strlen(str) * 2);
+ while (str && *str) {
+uint8_t c = *str++;
+char hexbuf[5];
+snprintf(hexbuf, sizeof(hexbuf), "%02x", c);
+hex_str += hexbuf;
+ }
+ return hex_str;
+}
+
+void append_hexified_string(std::ostream &ostrm, const std::string &string) {
+ size_t string_size = string.size();
+ const char *string_buf = string.c_str();
+ for (size_t i = 0; i < string_size; i++) {
+ostrm << RAWHEX8(*(string_buf + i));
+ }
+}
+
extern void ASLLogCallback(void *baton, uint32_t flags, const char *format,
va_list args);
@@ -171,7 +204,8 @@ RNBRemote::RNBRemote()
m_extended_mode(false), m_noack_mode(false),
m_thread_suffix_supported(false), m_list_threads_in_stop_reply(false),
m_compression_minsize(384), m_enable_compression_next_send_packet(false),
- m_compression_mode(compression_types::none) {
+ m_compression_mode(compression_types::none),
+ m_enable_error_strings(false) {
DNBLogThreadedIf(LOG_RNB_REMOTE, "%s", __PRETTY_FUNCTION__);
CreatePacketTable();
}
@@ -365,6 +399,11 @@ void RNBRemote::CreatePacketTable() {
t.push_back(Packet(
query_symbol_lookup, &RNBRemote::HandlePacket_qSymbol, NULL, "qSymbol:",
"Notify that host debugger is ready to do symbol lookups"));
+ t.push_back(Packet(enable_error_strings,
+ &RNBRemote::HandlePacket_QEnableErrorStri
[Lldb-commits] [lldb] 6757913 - [lldb][test] Fix PythonDataObjectsTest
Author: Jordan Rupprecht
Date: 2024-02-21T22:59:03-08:00
New Revision: 675791335285fa86434dc46e5c92f543e0e79d19
URL:
https://github.com/llvm/llvm-project/commit/675791335285fa86434dc46e5c92f543e0e79d19
DIFF:
https://github.com/llvm/llvm-project/commit/675791335285fa86434dc46e5c92f543e0e79d19.diff
LOG: [lldb][test] Fix PythonDataObjectsTest
This is using `FileSystem::Instance()` w/o calling `FileSystem::Initialize()`.
Use `SubsystemRAII` to do that.
Added:
Modified:
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
Removed:
diff --git
a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
index a4db4627f935b4..b90fbb7830995f 100644
--- a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -11,6 +11,7 @@
#include "Plugins/ScriptInterpreter/Python/PythonDataObjects.h"
#include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h"
+#include "TestingSupport/SubsystemRAII.h"
#include "lldb/Host/File.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/HostInfo.h"
@@ -26,6 +27,8 @@ using namespace lldb_private::python;
using llvm::Expected;
class PythonDataObjectsTest : public PythonTestSuite {
+ SubsystemRAII subsystems;
+
public:
void SetUp() override {
PythonTestSuite::SetUp();
@@ -209,8 +212,8 @@ TEST_F(PythonDataObjectsTest, TestPythonBoolean) {
};
// Test PythonBoolean constructed from long integer values.
- test_from_long(0); // Test 'false' value.
- test_from_long(1); // Test 'true' value.
+ test_from_long(0); // Test 'false' value.
+ test_from_long(1); // Test 'true' value.
test_from_long(~0); // Any value != 0 is 'true'.
}
@@ -811,7 +814,8 @@ main = foo
testing::ContainsRegex("line 7, in baz"),
testing::ContainsRegex("ZeroDivisionError");
-#if !((defined(_WIN32) || defined(_WIN64)) && (defined(__aarch64__) ||
defined(_M_ARM64)))
+#if !((defined(_WIN32) || defined(_WIN64)) &&
\
+ (defined(__aarch64__) || defined(_M_ARM64)))
static const char script2[] = R"(
class MyError(Exception):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPack… (PR #82593)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jason Molenda (jasonmolenda) Changes …etSupported Pavel added an extension to lldb's gdb remote serial protocol that allows the debug stub to append an error message (ascii hex encoded) after an error response packet Exx. This was added in 2017 in https://reviews.llvm.org/D34945 . lldb sends the QErrorStringInPacketSupported packet and then the remote stub may add these error strings. debugserver has added these strings to the vAttach family of packets to explain why an attach failed, without waiting to see the QErrorStringInPacketSupported packet to enable the mode. debugserver also has a bad implementation of this for the qLaunchSuccess packet, where "E" is sent followed by the literal characters of the error, instead of Exx;, which lldb can have problems parsing. This patch moves three utility functions earlier in RNBRemote.cpp, it accepts the QErrorStringInPacketSupported packet and only appends the expanded error messages when that has been sent (lldb always sends it at the beginning of a connection), fixes the error string returned by qLaunchSuccess and changes the vAttach error returns to only add the error string when the mode is enabled. --- Full diff: https://github.com/llvm/llvm-project/pull/82593.diff 2 Files Affected: - (modified) lldb/tools/debugserver/source/RNBRemote.cpp (+98-59) - (modified) lldb/tools/debugserver/source/RNBRemote.h (+5) ``diff diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp index feea4c914ec536..8338e4d0032870 100644 --- a/lldb/tools/debugserver/source/RNBRemote.cpp +++ b/lldb/tools/debugserver/source/RNBRemote.cpp @@ -143,6 +143,39 @@ uint64_t decode_uint64(const char *p, int base, char **end = nullptr, return addr; } +void append_hex_value(std::ostream &ostrm, const void *buf, size_t buf_size, + bool swap) { + int i; + const uint8_t *p = (const uint8_t *)buf; + if (swap) { +for (i = static_cast(buf_size) - 1; i >= 0; i--) + ostrm << RAWHEX8(p[i]); + } else { +for (size_t i = 0; i < buf_size; i++) + ostrm << RAWHEX8(p[i]); + } +} + +std::string cstring_to_asciihex_string(const char *str) { + std::string hex_str; + hex_str.reserve(strlen(str) * 2); + while (str && *str) { +uint8_t c = *str++; +char hexbuf[5]; +snprintf(hexbuf, sizeof(hexbuf), "%02x", c); +hex_str += hexbuf; + } + return hex_str; +} + +void append_hexified_string(std::ostream &ostrm, const std::string &string) { + size_t string_size = string.size(); + const char *string_buf = string.c_str(); + for (size_t i = 0; i < string_size; i++) { +ostrm << RAWHEX8(*(string_buf + i)); + } +} + extern void ASLLogCallback(void *baton, uint32_t flags, const char *format, va_list args); @@ -171,7 +204,8 @@ RNBRemote::RNBRemote() m_extended_mode(false), m_noack_mode(false), m_thread_suffix_supported(false), m_list_threads_in_stop_reply(false), m_compression_minsize(384), m_enable_compression_next_send_packet(false), - m_compression_mode(compression_types::none) { + m_compression_mode(compression_types::none), + m_enable_error_strings(false) { DNBLogThreadedIf(LOG_RNB_REMOTE, "%s", __PRETTY_FUNCTION__); CreatePacketTable(); } @@ -365,6 +399,11 @@ void RNBRemote::CreatePacketTable() { t.push_back(Packet( query_symbol_lookup, &RNBRemote::HandlePacket_qSymbol, NULL, "qSymbol:", "Notify that host debugger is ready to do symbol lookups")); + t.push_back(Packet(enable_error_strings, + &RNBRemote::HandlePacket_QEnableErrorStrings, NULL, + "QEnableErrorStrings", + "Tell " DEBUGSERVER_PROGRAM_NAME + " it can append descriptive error messages in replies.")); t.push_back(Packet(json_query_thread_extended_info, &RNBRemote::HandlePacket_jThreadExtendedInfo, NULL, "jThreadExtendedInfo", @@ -1566,11 +1605,13 @@ rnb_err_t RNBRemote::HandlePacket_qLaunchSuccess(const char *p) { if (m_ctx.HasValidProcessID() || m_ctx.LaunchStatus().Status() == 0) return SendPacket("OK"); std::ostringstream ret_str; - std::string status_str; - std::string error_quoted = binary_encode_string - (m_ctx.LaunchStatusAsString(status_str)); - ret_str << "E" << error_quoted; - + ret_str << "E89"; + if (m_enable_error_strings) { +std::string status_str; +std::string error_quoted = +cstring_to_asciihex_string(m_ctx.LaunchStatusAsString(status_str)); +ret_str << ";" << error_quoted; + } return SendPacket(ret_str.str()); } @@ -2534,39 +2575,6 @@ rnb_err_t RNBRemote::HandlePacket_QSetProcessEvent(const char *p) { return SendPacket("OK"); } -void append_hex_value(std::ostream &ostrm, const void *buf, size_t buf_size, - bool swap) { -
[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPack… (PR #82593)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff 386aa7b16977150da917a78423fd05cb19609850 0ba4e6402969028fa6152c366a56063a56acded1 -- lldb/tools/debugserver/source/RNBRemote.cpp lldb/tools/debugserver/source/RNBRemote.h `` View the diff from clang-format here. ``diff diff --git a/lldb/tools/debugserver/source/RNBRemote.h b/lldb/tools/debugserver/source/RNBRemote.h index 91f04fd6b4..b7b7d8262c 100644 --- a/lldb/tools/debugserver/source/RNBRemote.h +++ b/lldb/tools/debugserver/source/RNBRemote.h @@ -108,36 +108,36 @@ public: json_query_get_shared_cache_info, // 'jGetSharedCacheInfo' pass_signals_to_inferior, // 'QPassSignals' start_noack_mode, // 'QStartNoAckMode' -prefix_reg_packets_with_tid,// 'QPrefixRegisterPacketsWithThreadID -set_logging_mode, // 'QSetLogging:' -set_ignored_exceptions, // 'QSetIgnoredExceptions' -set_max_packet_size,// 'QSetMaxPacketSize:' -set_max_payload_size, // 'QSetMaxPayloadSize:' -set_environment_variable, // 'QEnvironment:' -set_environment_variable_hex, // 'QEnvironmentHexEncoded:' -set_launch_arch,// 'QLaunchArch:' -set_disable_aslr, // 'QSetDisableASLR:' -set_stdin, // 'QSetSTDIN:' -set_stdout, // 'QSetSTDOUT:' -set_stderr, // 'QSetSTDERR:' -set_working_dir,// 'QSetWorkingDir:' -set_list_threads_in_stop_reply, // 'QListThreadsInStopReply:' -sync_thread_state, // 'QSyncThreadState:' -memory_region_info, // 'qMemoryRegionInfo:' -get_profile_data, // 'qGetProfileData' -set_enable_profiling, // 'QSetEnableAsyncProfiling' -enable_compression, // 'QEnableCompression:' -watchpoint_support_info,// 'qWatchpointSupportInfo:' -allocate_memory,// '_M' -deallocate_memory, // '_m' -set_process_event, // 'QSetProcessEvent:' -save_register_state,// '_g' -restore_register_state, // '_G' -speed_test, // 'qSpeedTest:' -set_detach_on_error,// 'QSetDetachOnError:' -query_transfer, // 'qXfer:' -json_query_dyld_process_state, // 'jGetDyldProcessState' -enable_error_strings, // 'QEnableErrorStrings' +prefix_reg_packets_with_tid,// 'QPrefixRegisterPacketsWithThreadID +set_logging_mode, // 'QSetLogging:' +set_ignored_exceptions, // 'QSetIgnoredExceptions' +set_max_packet_size,// 'QSetMaxPacketSize:' +set_max_payload_size, // 'QSetMaxPayloadSize:' +set_environment_variable, // 'QEnvironment:' +set_environment_variable_hex, // 'QEnvironmentHexEncoded:' +set_launch_arch,// 'QLaunchArch:' +set_disable_aslr, // 'QSetDisableASLR:' +set_stdin, // 'QSetSTDIN:' +set_stdout, // 'QSetSTDOUT:' +set_stderr, // 'QSetSTDERR:' +set_working_dir,// 'QSetWorkingDir:' +set_list_threads_in_stop_reply, // 'QListThreadsInStopReply:' +sync_thread_state, // 'QSyncThreadState:' +memory_region_info, // 'qMemoryRegionInfo:' +get_profile_data, // 'qGetProfileData' +set_enable_profiling, // 'QSetEnableAsyncProfiling' +enable_compression, // 'QEnableCompression:' +watchpoint_support_info,// 'qWatchpointSupportInfo:' +allocate_memory,// '_M' +deallocate_memory, // '_m' +set_process_event, // 'QSetProcessEvent:' +save_register_state,// '_g' +restore_register_state, // '_G' +speed_test, // 'qSpeedTest:' +set_detach_on_error,// 'QSetDetachOnError:' +query_transfer, // 'qXfer:' +json_query_dyld_process_state, // 'jGetDyldProcessState' +enable_error_strings, // 'QEnableErrorStrings' unknown_type }; `` https://github.com/llvm/llvm-project/pull/82593 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Don't rely on ScriptInterpreterPythonImpl::Initialize in the unit tests (PR #82096)
@@ -9,43 +9,27 @@
#include "gtest/gtest.h"
#include "Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h"
-#include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h"
-#include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h"
#include "Plugins/ScriptInterpreter/Python/lldb-python.h"
-#include "lldb/Host/FileSystem.h"
-#include "lldb/Host/HostInfo.h"
-
#include "PythonTestSuite.h"
#include
-using namespace lldb_private;
-class TestScriptInterpreterPython : public ScriptInterpreterPythonImpl {
-public:
- using ScriptInterpreterPythonImpl::Initialize;
-};
-
void PythonTestSuite::SetUp() {
- FileSystem::Initialize();
rupprecht wrote:
FWIW, I think it's appropriate that `FileSystem::Initialize()` is removed here,
because `PythonTestSuite` itself does not seem to interact w/ `FileSystem`. But
`PythonDataObjectsTests.cpp` does, so it should be the one to take care of
`FileSystem::Initialize()`.
https://github.com/llvm/llvm-project/pull/82096
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Don't rely on ScriptInterpreterPythonImpl::Initialize in the unit tests (PR #82096)
rupprecht wrote: > This change has caused a failing test on Linux: > https://lab.llvm.org/buildbot/#/builders/68/builds/69157 > Fixed w/ 675791335285fa86434dc46e5c92f543e0e79d19 https://github.com/llvm/llvm-project/pull/82096 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
