Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")

[jingham]

Summary

Sometimes you want to make sure that the target doesn't run at all when running an expression, and you'd rather the expression fail if it would have run the target. You can do this with the "expression" command by passing "--allow-jit 0" but there's no way to do that with SBExpressionOptions. This patch adds that setting.

[aprantl]

This is great! Why not just call it --jit?

[jasonmolenda]

To me --jit sounds like an imperative (i.e. don't use the IR interpreter, jit and execute this expression), whereas --allow-jit seems closer to the behavior here.

[aprantl]

Ah, I didn't realize that the behavior is to always try to interpret first.

[jingham]

--jit wouldn't describe what the flag actually does. Currently allow-jit and the SB Setting I added for it set the execution policy to eExecutionPolicyWhenNeeded, not eExecutionPolicyAlways. So this really does just allow the JIT to be used, it doesn't force it.

If we wanted to add the ability to force use of the JIT always, we could either add another flag, or make an enum setting that had {never, always, when needed}. But I can't see why you would want that, except maybe to work around IR interpreter bugs. I'd rather not add options just for working around bugs if I can help it.

[jankratochvil]

It broke the testsuite for me:

$ time PYTHONPATH=$PWD/lib64/python2.7/site-packages:$PWD/lib64/python2.7/site-packages/lldb ../llvm-git/tools/lldb/test/dotest.py --executable $PWD/bin/lldb -C $PWD/bin/clang -t ../llvm-git/tools/lldb/packages/Python/lldbsuite/test/ 
WARNING:root:No valid FileCheck executable; some tests may fail...
WARNING:root:(Double-check the --filecheck argument to dotest.py)
LLDB library dir: /home/jkratoch/redhat/llvm-git-build-release-clang/bin
LLDB import library dir: /home/jkratoch/redhat/llvm-git-build-release-clang/bin
lldb version 8.0.0
  clang revision 6974b990e13dfb4190a6dffdcc8bac9edbd1cde5
  llvm revision 7fad5fb0d0d32beea4e95e239cc065a850733358
Libc++ tests will not be run because: Unable to find libc++ installation
Skipping following debug info categories: ['dsym', 'gmodules']
Traceback (most recent call last):
  File "../llvm-git/tools/lldb/test/dotest.py", line 7, in <module>
    lldbsuite.test.run_suite()
  File "/home/jkratoch/redhat/llvm-git/tools/lldb/packages/Python/lldbsuite/test/dotest.py", line 1323, in run_suite
    visit('Test', dirpath, filenames)
  File "/home/jkratoch/redhat/llvm-git/tools/lldb/packages/Python/lldbsuite/test/dotest.py", line 965, in visit
    raise Exception("Found multiple tests with the name %s" % name)
Exception: Found multiple tests with the name TestSampleTest.py

As really there are now:

$ find -name TestSampleTest.py
./packages/Python/lldbsuite/test/expression_command/dont_allow_jit/TestSampleTest.py
./packages/Python/lldbsuite/test/sample_test/TestSampleTest.py

[davide]

In D54056#1286620, @jankratochvil wrote:

It broke the testsuite for me:

$ time PYTHONPATH=$PWD/lib64/python2.7/site-packages:$PWD/lib64/python2.7/site-packages/lldb ../llvm-git/tools/lldb/test/dotest.py --executable $PWD/bin/lldb -C $PWD/bin/clang -t ../llvm-git/tools/lldb/packages/Python/lldbsuite/test/ 
WARNING:root:No valid FileCheck executable; some tests may fail...
WARNING:root:(Double-check the --filecheck argument to dotest.py)
LLDB library dir: /home/jkratoch/redhat/llvm-git-build-release-clang/bin
LLDB import library dir: /home/jkratoch/redhat/llvm-git-build-release-clang/bin
lldb version 8.0.0
  clang revision 6974b990e13dfb4190a6dffdcc8bac9edbd1cde5
  llvm revision 7fad5fb0d0d32beea4e95e239cc065a850733358
Libc++ tests will not be run because: Unable to find libc++ installation
Skipping following debug info categories: ['dsym', 'gmodules']
Traceback (most recent call last):
  File "../llvm-git/tools/lldb/test/dotest.py", line 7, in <module>
    lldbsuite.test.run_suite()
  File "/home/jkratoch/redhat/llvm-git/tools/lldb/packages/Python/lldbsuite/test/dotest.py", line 1323, in run_suite
    visit('Test', dirpath, filenames)
  File "/home/jkratoch/redhat/llvm-git/tools/lldb/packages/Python/lldbsuite/test/dotest.py", line 965, in visit
    raise Exception("Found multiple tests with the name %s" % name)
Exception: Found multiple tests with the name TestSampleTest.py

As really there are now:

$ find -name TestSampleTest.py
./packages/Python/lldbsuite/test/expression_command/dont_allow_jit/TestSampleTest.py
./packages/Python/lldbsuite/test/sample_test/TestSampleTest.py

I don't think anybody will look at this until Monday, so if you want a quick fix you might consider renaming the test yourself.

Davide

[jankratochvil]

In D54056#1286635, @davide wrote:

I don't think anybody will look at this until Monday, so if you want a quick fix you might consider renaming the test yourself.

Checked in as: rLLDB346089

[jingham]

Thanks for doing this! Why do we care about the file name of the test, shouldn't we be using the test class name for everything (that one I did remember to change...)

Jim

lldb/trunk/include/lldb/API/SBExpressionOptions.h

public:
 
   void SetAutoApplyFixIts(bool b = true);
 
   bool GetAutoApplyFixIts();
 
   bool GetTopLevel();
 
   void SetTopLevel(bool b = true);
   
+  // Gets whether we will JIT an expression if it cannot be interpreted
+  bool GetAllowJIT();
+  
+  // Sets whether we will JIT an expression if it cannot be interpreted
+  void SetAllowJIT(bool allow);
+
 protected:
   SBExpressionOptions(
       lldb_private::EvaluateExpressionOptions &expression_options);
 
   lldb_private::EvaluateExpressionOptions *get() const;
 
   lldb_private::EvaluateExpressionOptions &ref() const;
 

lldb/trunk/packages/Python/lldbsuite/test/expression_command/dont_allow_jit/Makefile

+LEVEL = ../../make
+
+C_SOURCES := main.c
+CFLAGS_EXTRAS += -std=c99
+
+include $(LEVEL)/Makefile.rules

lldb/trunk/packages/Python/lldbsuite/test/expression_command/dont_allow_jit/TestSampleTest.py

+"""
+Test that --allow-jit=false does disallow JITting:
+"""
+
+from __future__ import print_function
+
+
+import os
+import time
+import re
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+class TestAllowJIT(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    # If your test case doesn't stress debug info, the
+    # set this to true.  That way it won't be run once for
+    # each debug info format.
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr21765")
+    def test_allow_jit_expr_command(self):
+        """Test the --allow-jit command line flag"""
+        self.build()
+        self.main_source_file = lldb.SBFileSpec("main.c")
+        self.expr_cmd_test()
+
+    @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr21765")
+    def test_allow_jit_options(self):
+        """Test the SetAllowJIT SBExpressionOption setting"""
+        self.build()
+        self.main_source_file = lldb.SBFileSpec("main.c")
+        self.expr_options_test()
+
+        
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+
+    def expr_cmd_test(self):
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+                                   "Set a breakpoint here", self.main_source_file)
+
+        frame = thread.GetFrameAtIndex(0)
+        
+        # First make sure we can call the function with 
+        interp = self.dbg.GetCommandInterpreter()
+        self.expect("expr --allow-jit 1 -- call_me(10)",
+                    substrs = ["(int) $", "= 18"])
+        # Now make sure it fails with the "can't IR interpret message" if allow-jit is false:
+        self.expect("expr --allow-jit 0 -- call_me(10)",
+                    error=True,
+                    substrs = ["Can't run the expression locally"])
+        
+    def expr_options_test(self):
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+                                   "Set a breakpoint here", self.main_source_file)
+
+        frame = thread.GetFrameAtIndex(0)
+        
+        # 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")
+
+        # Now use the options:
+        result = frame.EvaluateExpression("call_me(10)", options)
+        self.assertTrue(result.GetError().Success(), "expression succeeded")
+        self.assertEqual(result.GetValueAsSigned(), 18, "got the right value.")
+
+        # 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")
+
+        # Again use it and ensure we fail:
+        result = frame.EvaluateExpression("call_me(10)", options)
+        self.assertTrue(result.GetError().Fail(), "expression failed with no JIT")
+        self.assertTrue("Can't run the expression locally" in result.GetError().GetCString(), "Got right error")
+
+        # Finally set the allow JIT value back to true and make sure that works:
+        options.SetAllowJIT(True)
+        self.assertEqual(options.GetAllowJIT(), True, "Set back to True correctly")
+
+        # And again, make sure this works:
+        result = frame.EvaluateExpression("call_me(10)", options)
+        self.assertTrue(result.GetError().Success(), "expression succeeded")
+        self.assertEqual(result.GetValueAsSigned(), 18, "got the right value.")
+

lldb/trunk/packages/Python/lldbsuite/test/expression_command/dont_allow_jit/main.c

+#include <stdio.h>
+
+int
+call_me(int input)
+{
+  return printf("I was called: %d.\n", input);
+}
+
+int
+main()
+{
+  int test_var = 10;
+  printf ("Set a breakpoint here: %d.\n", test_var);
+  return call_me(100);
+}

lldb/trunk/scripts/interface/SBExpressionOptions.i

public:
     bool
     GetAutoApplyFixIts();
 
     bool
     GetTopLevel();
 
     void
     SetTopLevel(bool b = true);
   
+    %feature("docstring", "Gets whether to JIT an expression if it cannot be interpreted.") GetAllowJIT;
+    bool
+    GetAllowJIT();
+  
+    %feature("docstring", "Sets whether to JIT an expression if it cannot be interpreted.") SetAllowJIT;
+    void
+    SetAllowJIT(bool allow);
+
 protected:
 
     SBExpressionOptions (lldb_private::EvaluateExpressionOptions &expression_options);
 
     lldb_private::EvaluateExpressionOptions *
     get () const;
 
     lldb_private::EvaluateExpressionOptions &
     ref () const;
 
 private:
     // This auto_pointer is made in the constructor and is always valid.
     mutable std::unique_ptr<lldb_private::EvaluateExpressionOptions> m_opaque_ap;
 };
 
 } // namespace lldb

lldb/trunk/source/API/SBExpressionOptions.cpp

bool SBExpressionOptions::GetTopLevel() {
   return m_opaque_ap->GetExecutionPolicy() == eExecutionPolicyTopLevel;
 }
 
 void SBExpressionOptions::SetTopLevel(bool b) {
   m_opaque_ap->SetExecutionPolicy(b ? eExecutionPolicyTopLevel
                                     : m_opaque_ap->default_execution_policy);
 }
 
+bool SBExpressionOptions::GetAllowJIT() {
+  return m_opaque_ap->GetExecutionPolicy() != eExecutionPolicyNever;
+}
+
+void SBExpressionOptions::SetAllowJIT(bool allow) {
+  m_opaque_ap->SetExecutionPolicy(allow ? m_opaque_ap->default_execution_policy
+                                    : eExecutionPolicyNever);
+}
+
 EvaluateExpressionOptions *SBExpressionOptions::get() const {
   return m_opaque_ap.get();
 }
 
 EvaluateExpressionOptions &SBExpressionOptions::ref() const {
   return *(m_opaque_ap.get());
 }