Re: [Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")
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 > On Nov 3, 2018, at 11:18 PM, Jan Kratochvil via Phabricator > wrote: > > jankratochvil added a comment. > > In https://reviews.llvm.org/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: https://reviews.llvm.org/rLLDB346089 > > > Repository: > rL LLVM > > https://reviews.llvm.org/D54056 > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")
jingham added a subscriber: aprantl. jingham added a comment. 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 Repository: rL LLVM https://reviews.llvm.org/D54056 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")
jankratochvil added a comment. In https://reviews.llvm.org/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: https://reviews.llvm.org/rLLDB346089 Repository: rL LLVM https://reviews.llvm.org/D54056 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")
davide added a comment. In https://reviews.llvm.org/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 > 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 Repository: rL LLVM https://reviews.llvm.org/D54056 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")
jankratochvil added a comment. 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 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 Repository: rL LLVM https://reviews.llvm.org/D54056 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")
This revision was automatically updated to reflect the committed changes. Closed by commit rL346053: Add an SBExpressionOptions setting mirroring the exec commands --allow-jit. (authored by jingham, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D54056?vs=172454=172465#toc Repository: rL LLVM https://reviews.llvm.org/D54056 Files: lldb/trunk/include/lldb/API/SBExpressionOptions.h lldb/trunk/packages/Python/lldbsuite/test/expression_command/dont_allow_jit/Makefile lldb/trunk/packages/Python/lldbsuite/test/expression_command/dont_allow_jit/TestSampleTest.py lldb/trunk/packages/Python/lldbsuite/test/expression_command/dont_allow_jit/main.c lldb/trunk/scripts/interface/SBExpressionOptions.i lldb/trunk/source/API/SBExpressionOptions.cpp Index: lldb/trunk/packages/Python/lldbsuite/test/expression_command/dont_allow_jit/Makefile === --- lldb/trunk/packages/Python/lldbsuite/test/expression_command/dont_allow_jit/Makefile +++ lldb/trunk/packages/Python/lldbsuite/test/expression_command/dont_allow_jit/Makefile @@ -0,0 +1,6 @@ +LEVEL = ../../make + +C_SOURCES := main.c +CFLAGS_EXTRAS += -std=c99 + +include $(LEVEL)/Makefile.rules Index: lldb/trunk/packages/Python/lldbsuite/test/expression_command/dont_allow_jit/TestSampleTest.py === --- lldb/trunk/packages/Python/lldbsuite/test/expression_command/dont_allow_jit/TestSampleTest.py +++ lldb/trunk/packages/Python/lldbsuite/test/expression_command/dont_allow_jit/TestSampleTest.py @@ -0,0 +1,94 @@ +""" +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
[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")
jingham added a comment. --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. Repository: rLLDB LLDB https://reviews.llvm.org/D54056 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")
aprantl added a comment. Ah, I didn't realize that the behavior is to always try to interpret first. Repository: rLLDB LLDB https://reviews.llvm.org/D54056 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")
jasonmolenda added a comment. 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. Repository: rLLDB LLDB https://reviews.llvm.org/D54056 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. This is great! Why not just call it `--jit`? Repository: rLLDB LLDB https://reviews.llvm.org/D54056 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")
jingham created this revision. jingham added a reviewer: aprantl. Herald added a subscriber: lldb-commits. 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. Repository: rLLDB LLDB https://reviews.llvm.org/D54056 Files: include/lldb/API/SBExpressionOptions.h packages/Python/lldbsuite/test/expression_command/dont_allow_jit/Makefile packages/Python/lldbsuite/test/expression_command/dont_allow_jit/TestSampleTest.py packages/Python/lldbsuite/test/expression_command/dont_allow_jit/main.c scripts/interface/SBExpressionOptions.i source/API/SBExpressionOptions.cpp Index: source/API/SBExpressionOptions.cpp === --- source/API/SBExpressionOptions.cpp +++ source/API/SBExpressionOptions.cpp @@ -159,6 +159,15 @@ : 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(); } Index: scripts/interface/SBExpressionOptions.i === --- scripts/interface/SBExpressionOptions.i +++ scripts/interface/SBExpressionOptions.i @@ -132,6 +132,14 @@ 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: Index: packages/Python/lldbsuite/test/expression_command/dont_allow_jit/main.c === --- packages/Python/lldbsuite/test/expression_command/dont_allow_jit/main.c +++ packages/Python/lldbsuite/test/expression_command/dont_allow_jit/main.c @@ -0,0 +1,15 @@ +#include + +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); +} Index: packages/Python/lldbsuite/test/expression_command/dont_allow_jit/TestSampleTest.py === --- packages/Python/lldbsuite/test/expression_command/dont_allow_jit/TestSampleTest.py +++ packages/Python/lldbsuite/test/expression_command/dont_allow_jit/TestSampleTest.py @@ -0,0 +1,94 @@ +""" +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