Page MenuHomePhabricator

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

Authored by jingham on Nov 2 2018, 4:06 PM.

Details

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.

Diff Detail

Repository
rLLDB LLDB

Event Timeline

jingham created this revision.Nov 2 2018, 4:06 PM
aprantl accepted this revision.Nov 2 2018, 4:23 PM

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

This revision is now accepted and ready to land.Nov 2 2018, 4:23 PM

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.

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

--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.

This revision was automatically updated to reflect the committed changes.

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 added a subscriber: davide.Nov 3 2018, 3:37 PM

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

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

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