[Lldb-commits] [lldb] r373201 - [lldb] Reland 370734: Test 'frame select -r' and fix that INT32_MIN breaks the option parser

Raphael Isemann via lldb-commits lldb-commits at lists.llvm.org
Mon Sep 30 05:49:32 PDT 2019


Author: teemperor
Date: Mon Sep 30 05:49:32 2019
New Revision: 373201

URL: http://llvm.org/viewvc/llvm-project?rev=373201&view=rev
Log:
[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.

Modified:
    lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py
    lldb/trunk/source/Commands/CommandObjectFrame.cpp

Modified: lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py?rev=373201&r1=373200&r2=373201&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py Mon Sep 30 05:49:32 2019
@@ -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"])

Modified: lldb/trunk/source/Commands/CommandObjectFrame.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectFrame.cpp?rev=373201&r1=373200&r2=373201&view=diff
==============================================================================
--- lldb/trunk/source/Commands/CommandObjectFrame.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectFrame.cpp Mon Sep 30 05:49:32 2019
@@ -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<OptionDefinition> GetDefinitions() override {
       return llvm::makeArrayRef(g_frame_select_options);
     }
 
-    int32_t relative_frame_offset;
+    llvm::Optional<int32_t> 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<int32_t>(frame_idx) >= -m_options.relative_frame_offset)
-          frame_idx += m_options.relative_frame_offset;
+      if (*m_options.relative_frame_offset < 0) {
+        if (static_cast<int32_t>(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<int32_t>(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




More information about the lldb-commits mailing list