From dab6f0746d17dc087a846c2d42f380c98e5513fe Mon Sep 17 00:00:00 2001 From: Raphael Isemann Date: Mon, 30 Sep 2019 12:49:32 +0000 Subject: [PATCH] [lldb] Reland 370734: Test 'frame select -r' and fix that INT32_MIN breaks the option parser The problem with r370734 was that it removed the code for resetting the options in OptionParsingStarting. This caused that once a 'frame select -r ...' command was executed, we kept the relative index argument for all following 'frame select ...' invocations (even the ones with an absolute index as they are the same command object). See rdar://55791276. This relands the patch but keeps the code that resets the command options before execution. llvm-svn: 373201 --- .../test/commands/frame/select/TestFrameSelect.py | 43 ++++++++++++++++++++++ lldb/source/Commands/CommandObjectFrame.cpp | 29 ++++++++------- 2 files changed, 59 insertions(+), 13 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py b/lldb/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py index 6746aaf..2f2dd5a 100644 --- a/lldb/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py +++ b/lldb/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py @@ -23,12 +23,55 @@ class TestFrameSelect(TestBase): self.expect("frame select -r -1", error=True, substrs=["Already at the bottom of the stack."]) self.expect("frame select -r -2147483647", error=True, substrs=["Already at the bottom of the stack."]) + self.expect("frame select -r -2147483648", error=True, substrs=["error: invalid frame offset argument '-2147483648'"]) + self.expect("frame select -r -2147483649", error=True, substrs=["error: invalid frame offset argument '-2147483649'"]) self.expect("frame select -r 1", substrs=["nested2() at"]) self.expect("frame select -r -2", substrs=["nested3() at"]) self.expect("frame select -r 1", substrs=["nested2() at"]) self.expect("frame select -r -2147483647", substrs=["nested3() at"]) self.expect("frame select -r 1", substrs=["nested2() at"]) + self.expect("frame select -r -2147483648", error=True, substrs=["error: invalid frame offset argument '-2147483648'"]) + self.expect("frame select -r -2147483649", error=True, substrs=["error: invalid frame offset argument '-2147483649'"]) self.expect("frame select -r 100") self.expect("frame select -r 1", error=True, substrs=["Already at the top of the stack."]) + + @no_debug_info_test + @skipIfWindows + def test_mixing_relative_and_abs(self): + self.build() + + lldbutil.run_to_source_breakpoint(self, + "// Set break point at this line.", lldb.SBFileSpec("main.cpp")) + + # The function associated with each frame index can change depending + # on the function calling main (e.g. `start`), so this only tests that + # the frame index number is correct. We test the actual functions + # in the relative test. + + # Jump to the top of the stack. + self.expect("frame select 0", substrs=["frame #0"]) + + # Run some relative commands. + self.expect("up", substrs=["frame #1"]) + self.expect("frame select -r 1", substrs=["frame #2"]) + self.expect("frame select -r -1", substrs=["frame #1"]) + + # Test that absolute indices still work. + self.expect("frame select 2", substrs=["frame #2"]) + self.expect("frame select 1", substrs=["frame #1"]) + self.expect("frame select 3", substrs=["frame #3"]) + self.expect("frame select 0", substrs=["frame #0"]) + self.expect("frame select 1", substrs=["frame #1"]) + + # Run some other relative frame select commands. + self.expect("down", substrs=["frame #0"]) + self.expect("frame select -r 1", substrs=["frame #1"]) + self.expect("frame select -r -1", substrs=["frame #0"]) + + # Test that absolute indices still work. + self.expect("frame select 2", substrs=["frame #2"]) + self.expect("frame select 1", substrs=["frame #1"]) + self.expect("frame select 3", substrs=["frame #3"]) + self.expect("frame select 0", substrs=["frame #0"]) diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp index 0a10231..6a7facd 100644 --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -246,13 +246,15 @@ public: Status error; const int short_option = m_getopt_table[option_idx].val; switch (short_option) { - case 'r': - if (option_arg.getAsInteger(0, relative_frame_offset)) { - relative_frame_offset = INT32_MIN; + case 'r': { + int32_t offset = 0; + if (option_arg.getAsInteger(0, offset) || offset == INT32_MIN) { error.SetErrorStringWithFormat("invalid frame offset argument '%s'", option_arg.str().c_str()); - } + } else + relative_frame_offset = offset; break; + } default: llvm_unreachable("Unimplemented option"); @@ -262,14 +264,14 @@ public: } void OptionParsingStarting(ExecutionContext *execution_context) override { - relative_frame_offset = INT32_MIN; + relative_frame_offset.reset(); } llvm::ArrayRef GetDefinitions() override { return llvm::makeArrayRef(g_frame_select_options); } - int32_t relative_frame_offset; + llvm::Optional relative_frame_offset; }; CommandObjectFrameSelect(CommandInterpreter &interpreter) @@ -307,15 +309,16 @@ protected: Thread *thread = m_exe_ctx.GetThreadPtr(); uint32_t frame_idx = UINT32_MAX; - if (m_options.relative_frame_offset != INT32_MIN) { + if (m_options.relative_frame_offset.hasValue()) { // The one and only argument is a signed relative frame index frame_idx = thread->GetSelectedFrameIndex(); if (frame_idx == UINT32_MAX) frame_idx = 0; - if (m_options.relative_frame_offset < 0) { - if (static_cast(frame_idx) >= -m_options.relative_frame_offset) - frame_idx += m_options.relative_frame_offset; + if (*m_options.relative_frame_offset < 0) { + if (static_cast(frame_idx) >= + -*m_options.relative_frame_offset) + frame_idx += *m_options.relative_frame_offset; else { if (frame_idx == 0) { // If you are already at the bottom of the stack, then just warn @@ -326,15 +329,15 @@ protected: } else frame_idx = 0; } - } else if (m_options.relative_frame_offset > 0) { + } else if (*m_options.relative_frame_offset > 0) { // I don't want "up 20" where "20" takes you past the top of the stack // to produce // an error, but rather to just go to the top. So I have to count the // stack here... const uint32_t num_frames = thread->GetStackFrameCount(); if (static_cast(num_frames - frame_idx) > - m_options.relative_frame_offset) - frame_idx += m_options.relative_frame_offset; + *m_options.relative_frame_offset) + frame_idx += *m_options.relative_frame_offset; else { if (frame_idx == num_frames - 1) { // If we are already at the top of the stack, just warn and don't -- 2.7.4