Re: [Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")

2018-11-05 Thread Jim Ingham via lldb-commits
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")

2018-11-05 Thread Jim Ingham via Phabricator via lldb-commits
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")

2018-11-04 Thread Jan Kratochvil via Phabricator via lldb-commits
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")

2018-11-03 Thread Davide Italiano via Phabricator via lldb-commits
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")

2018-11-03 Thread Jan Kratochvil via Phabricator via lldb-commits
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")

2018-11-02 Thread Jim Ingham via Phabricator via lldb-commits
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")

2018-11-02 Thread Jim Ingham via Phabricator via lldb-commits
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")

2018-11-02 Thread Adrian Prantl via Phabricator via lldb-commits
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")

2018-11-02 Thread Jason Molenda via Phabricator via lldb-commits
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")

2018-11-02 Thread Adrian Prantl via Phabricator via lldb-commits
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")

2018-11-02 Thread Jim Ingham via Phabricator via lldb-commits
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