[Lldb-commits] [lldb] 54c496d - [lldb] Allow to register frame recognizers applied beyond the first instruction

Med Ismail Bennani via lldb-commits lldb-commits at lists.llvm.org
Sun Aug 29 08:37:59 PDT 2021


Author: Roman Podoliaka
Date: 2021-08-29T17:28:46+02:00
New Revision: 54c496dad6f28f1ab663f1b30401fe460709d50d

URL: https://github.com/llvm/llvm-project/commit/54c496dad6f28f1ab663f1b30401fe460709d50d
DIFF: https://github.com/llvm/llvm-project/commit/54c496dad6f28f1ab663f1b30401fe460709d50d.diff

LOG: [lldb] Allow to register frame recognizers applied beyond the first instruction

It is currently possible to register a frame recognizer, but it will be applied if and only if the frame's PC points to the very first instruction of the specified function, which limits usability of this feature.

The implementation already supports changing this behaviour by passing an additional flag, but it's not possible to set it via the command interface. Fix that.

Reviewed By: jingham

Differential Revision: https://reviews.llvm.org/D108510

Added: 
    

Modified: 
    lldb/source/Commands/CommandObjectFrame.cpp
    lldb/source/Commands/Options.td
    lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py

Removed: 
    


################################################################################
diff  --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp
index d90e357bf1aa3..1fd36e65ae151 100644
--- a/lldb/source/Commands/CommandObjectFrame.cpp
+++ b/lldb/source/Commands/CommandObjectFrame.cpp
@@ -14,6 +14,7 @@
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Interpreter/OptionGroupFormat.h"
 #include "lldb/Interpreter/OptionGroupValueObjectDisplay.h"
 #include "lldb/Interpreter/OptionGroupVariable.h"
@@ -739,6 +740,17 @@ class CommandObjectFrameRecognizerAdd : public CommandObjectParsed {
       const int short_option = m_getopt_table[option_idx].val;
 
       switch (short_option) {
+      case 'f': {
+        bool value, success;
+        value = OptionArgParser::ToBoolean(option_arg, true, &success);
+        if (success) {
+          m_first_instruction_only = value;
+        } else {
+          error.SetErrorStringWithFormat(
+              "invalid boolean value '%s' passed for -f option",
+              option_arg.str().c_str());
+        }
+      } break;
       case 'l':
         m_class_name = std::string(option_arg);
         break;
@@ -763,6 +775,7 @@ class CommandObjectFrameRecognizerAdd : public CommandObjectParsed {
       m_symbols.clear();
       m_class_name = "";
       m_regex = false;
+      m_first_instruction_only = true;
     }
 
     llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
@@ -774,6 +787,7 @@ class CommandObjectFrameRecognizerAdd : public CommandObjectParsed {
     std::string m_module;
     std::vector<std::string> m_symbols;
     bool m_regex;
+    bool m_first_instruction_only;
   };
 
   CommandOptions m_options;
@@ -883,13 +897,13 @@ bool CommandObjectFrameRecognizerAdd::DoExecute(Args &command,
     auto func =
         RegularExpressionSP(new RegularExpression(m_options.m_symbols.front()));
     GetSelectedOrDummyTarget().GetFrameRecognizerManager().AddRecognizer(
-        recognizer_sp, module, func);
+        recognizer_sp, module, func, m_options.m_first_instruction_only);
   } else {
     auto module = ConstString(m_options.m_module);
     std::vector<ConstString> symbols(m_options.m_symbols.begin(),
                                      m_options.m_symbols.end());
     GetSelectedOrDummyTarget().GetFrameRecognizerManager().AddRecognizer(
-        recognizer_sp, module, symbols);
+        recognizer_sp, module, symbols, m_options.m_first_instruction_only);
   }
 #endif
 

diff  --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index a766d79065f4c..5cef8f93b6989 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -404,6 +404,13 @@ let Command = "frame recognizer add" in {
     Desc<"Give the name of a Python class to use for this frame recognizer.">;
   def frame_recognizer_regex : Option<"regex", "x">,
     Desc<"Function name and module name are actually regular expressions.">;
+  def frame_recognizer_first_instruction_only : Option<"first-instruction-only", "f">, Arg<"Boolean">,
+    Desc<"If true, only apply this recognizer to frames whose PC currently points to the "
+    "first instruction of the specified function. If false, the recognizer "
+    "will always be applied, regardless of the current position within the specified function. The "
+    "implementor should keep in mind that some features, e.g. accessing function argument "
+    "values via $arg<N>, are not guaranteed to work reliably in this case, so extra care must "
+    "be taken to make the recognizer operate correctly. Defaults to true.">;
 }
 
 let Command = "history" in {

diff  --git a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
index b025a2255f07b..284aeed701699 100644
--- a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
+++ b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
@@ -205,6 +205,64 @@ def test_frame_recognizer_target_specific(self):
         self.expect("frame recognizer info 0",
                     substrs=['frame 0 is recognized by recognizer.MyFrameRecognizer'])
 
+    @skipUnlessDarwin
+    def test_frame_recognizer_not_only_first_instruction(self):
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+
+        # Clear internal & plugins recognizers that get initialized at launch.
+        self.runCmd("frame recognizer clear")
+
+        self.runCmd("command script import " + os.path.join(self.getSourceDir(), "recognizer.py"))
+
+        self.expect("frame recognizer list",
+                    substrs=['no matching results found.'])
+
+        # Create a target.
+        target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "foo",
+                                                                 exe_name = exe)
+
+        # Move the PC one instruction further.
+        self.runCmd("next")
+
+        # Add a frame recognizer in that target.
+        self.runCmd("frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo -n bar")
+
+        # It's not applied to foo(), because frame's PC is not at the first instruction of the function.
+        self.expect("frame recognizer info 0",
+                    substrs=['frame 0 not recognized by any recognizer'])
+
+        # Add a frame recognizer with --first-instruction-only=true.
+        self.runCmd("frame recognizer clear")
+
+        self.runCmd("frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo -n bar --first-instruction-only=true")
+
+        # It's not applied to foo(), because frame's PC is not at the first instruction of the function.
+        self.expect("frame recognizer info 0",
+                    substrs=['frame 0 not recognized by any recognizer'])
+
+        # Now add a frame recognizer with --first-instruction-only=false.
+        self.runCmd("frame recognizer clear")
+
+        self.runCmd("frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo -n bar --first-instruction-only=false")
+
+        # This time it should recognize the frame.
+        self.expect("frame recognizer info 0",
+                    substrs=['frame 0 is recognized by recognizer.MyFrameRecognizer'])
+
+        opts = lldb.SBVariablesOptions()
+        opts.SetIncludeRecognizedArguments(True)
+        frame = thread.GetSelectedFrame()
+        variables = frame.GetVariables(opts)
+
+        self.assertEqual(variables.GetSize(), 2)
+        self.assertEqual(variables.GetValueAtIndex(0).name, "a")
+        self.assertEqual(variables.GetValueAtIndex(0).signed, 42)
+        self.assertEqual(variables.GetValueAtIndex(0).GetValueType(), lldb.eValueTypeVariableArgument)
+        self.assertEqual(variables.GetValueAtIndex(1).name, "b")
+        self.assertEqual(variables.GetValueAtIndex(1).signed, 56)
+        self.assertEqual(variables.GetValueAtIndex(1).GetValueType(), lldb.eValueTypeVariableArgument)
+
     @no_debug_info_test
     def test_frame_recognizer_delete_invalid_arg(self):
         self.expect("frame recognizer delete a", error=True,
@@ -226,3 +284,12 @@ def test_frame_recognizer_info_invalid_arg(self):
                     substrs=["error: '-1' is not a valid frame index."])
         self.expect("frame recognizer info 4294967297", error=True,
                     substrs=["error: '4294967297' is not a valid frame index."])
+
+    @no_debug_info_test
+    def test_frame_recognizer_add_invalid_arg(self):
+        self.expect("frame recognizer add -f", error=True,
+                    substrs=["error: last option requires an argument"])
+        self.expect("frame recognizer add -f -1", error=True,
+                    substrs=["error: invalid boolean value '-1' passed for -f option"])
+        self.expect("frame recognizer add -f foo", error=True,
+                    substrs=["error: invalid boolean value 'foo' passed for -f option"])


        


More information about the lldb-commits mailing list