Open Bug 1590664 Opened 5 years ago Updated 2 years ago

./mach jstests ignores '--debugger' argument

Categories

(Testing :: General, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: allstars.chh, Unassigned)

Details

(Whiteboard: dev-prod-2020)

I cannot specify debugger when running ./mach jstests.
I have to do it with jstests.py -g --debugger ...

Component: Mach Core → General
Product: Firefox Build System → Testing
Priority: -- → P3

I'll take a look, possibly a fallout from my changes in 1519369 and 1573585.

:allstarschh - I'm attempting to step through the debugger, but I keep getting this:

(python27_development) egao@egao-44538:~/src/mozilla-central$ ./mach jstests --debugger=ls
Could not find executable shell: /Users/egao/src/mozilla-central/objdir-desktop-artifact/dist/bin/js

How could I get around this issue?


As for the cause, I suspect it is possible the addition of --debugger argument in get_test_parser() is interfering with the --debugger argument in jstests.py, but I cannot prove that this is the case yet.

Flags: needinfo?(allstars.chh)

(python27_development) egao@egao-44538:~/src/mozilla-central$ ./mach jstests --debugger=ls
Could not find executable shell: /Users/egao/src/mozilla-central/objdir-desktop-artifact/dist/bin/js


How could I get around this issue?

you could enable js shell by adding
ac_add_options --enable-js-shell in the mozconfig

or see
https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Build_Documentation
to build it manually(In reply to Edwin Takahashi (:egao, :etakahashi) from comment #2)

Flags: needinfo?(allstars.chh)

I did a full build after adding ac_add_options --enable-js-shell to my mozconfig.

In my investigation, I added the two lines print(options); sys.exit(1) to this line in order to check the contents of the parsed arguments, and I got this:

{'debugger': 'ls', 'show_cmd': None, 'passthrough': None, 'worker_count': 4, 'random': None, 'run_only_skipped': None, 'xul_info_src': None, 'rr': None, 'exclude_file': None, 'output_file': None, 'failure_file': None, 'slow_test_threshold': 5.0, 'jorendb': None, 'tbpl': None, 'js_shell': '/Users/egao/mozilla-central/objdir-desktop-artifact/dist/bin/js', 'repeat': 1, 'test_file': None, 'excluded_paths': [], 'format': 'none', 'show_slow': None, 'wptreport': None, 'tbpl_debug': None, 'feature_args': '', 'run_slow_tests': None, 'jitflags': 'jstests', 'output_fp': <open file '<stdout>', mode 'w' at 0x10cf97150>, 'show_output': None, 'run_skipped': None, 'no_extensions': None, 'wpt': 'if-running-everything', 'test_reflect_stringify': None, 'hide_progress': None, 'requested_paths': [], 'make_manifests': None, 'shell_args': '', 'valgrind': None, 'check_output': None, 'valgrind_args': '', 'failed_only': None, 'timeout': 150.0, 'debug': None, 'no_show_failed': None}

So it looks like at least the parsed arguments are interpreting the debugger parameter correctly.

After some more investigation and comparing the two methods of invoking jstests provided by :allstarschh, I may have some answers.

This method works to properly invoke debugger:
./js/src/tests/jstests.py --tinderbox obj-x86_64-pc-linux-gnu/dist/bin/js built-ins/WeakMap/constructor.js --debugger=rr -g

This method was reported in this bug as not invoking the debugger:
./mach jstests --debugger ls js/src/tests/test262/built-ins/Array/prototype/forEach/15.4.4.18-7-c-ii-1.js

I found that the key difference here, and the reason why the second method didn't work was the -g flag.

When the parser for jstests is being built, there is a parameter for --debug which is set to False by default. If I had written the script, I would have set the debug flag to True if any of the following flags were not the default value:

  • --debugger
  • --rr
  • --valgrind

But that is not what's happening here; the debug flag needs to be manually set to True by passing in the --debug/-g parameter. Which means, the actual command when using mach should be:

./mach jstests --debugger ls js/src/tests/test262/built-ins/Array/prototype/forEach/15.4.4.18-7-c-ii-1.js -g
or
./mach jstests --debugger ls js/src/tests/test262/built-ins/Array/prototype/forEach/15.4.4.18-7-c-ii-1.js --debug

I was able to successfully invoke my debugger (ls) without making any changes to the code:

[u'/Users/egao/mozilla-central/objdir-desktop-artifact/_virtualenvs/init/bin/python', u'/Users/egao/mozilla-central/js/src/tests/jstests.py', u'/Users/egao/mozilla-central/objdir-desktop-artifact/dist/bin/js', u'--jitflags=jstests', '--debugger', 'ls', '-g', 'js/src/tests/test262/built-ins/Array/prototype/forEach/15.4.4.18-7-c-ii-1.js']
> /Users/egao/mozilla-central/testing/mach_commands.py(538)run_jstests()
-> return subprocess.call(jstest_cmd)
(Pdb) c
> /Users/egao/mozilla-central/js/src/tests/jstests.py(538)main()
-> options, prefix, requested_paths, excluded_paths = parse_args()
(Pdb) c
> /Users/egao/mozilla-central/js/src/tests/jstests.py(243)parse_args()
-> if options.debug:
(Pdb) c
line 246:  ['ls']

I could make a change in jstests.py to remove this redundant check after the parameters are parsed, to check for the presence of at least one of the debugger parameter and to automatically set that flag to True. Let me know if that's what you'd want.

Flags: needinfo?(allstars.chh)

(In reply to Edwin Takahashi (:egao, :etakahashi) from comment #5)

But that is not what's happening here; the debug flag needs to be manually set to True by passing in the --debug/-g parameter. Which means, the actual command when using mach should be:

./mach jstests --debugger ls js/src/tests/test262/built-ins/Array/prototype/forEach/15.4.4.18-7-c-ii-1.js -g
or
./mach jstests --debugger ls js/src/tests/test262/built-ins/Array/prototype/forEach/15.4.4.18-7-c-ii-1.js --debug

I was able to successfully invoke my debugger (ls) without making any changes to the code:

Cool, so jstests already accepts debugger argument, then it would be great to show this in help
./mach help jstests
because in the first place I thought ./mach jstests doesn't accept debugger argument hence filed this bug.

I could make a change in jstests.py to remove this redundant check after the parameters are parsed, to check for the presence of at least one of the debugger parameter and to automatically set that flag to True. Let me know if that's what you'd want.

I am not sure the design of jstests.py, but at the moment I think we just do the same way as jstests.py, so if we need the '-g' in jstests.py, then ./mach jstests should also behave the same.

With your information,
now I added '-g' when running ./mach jstests, it shows some error message

./mach jstests built-ins/WeakMap/constructor.js --debugger gdb -g
Multiple tests match command line arguments, debugger can only run one
    test262/built-ins/WeakMap/constructor.js
    test262/built-ins/WeakMap/constructor.js
    test262/built-ins/WeakMap/constructor.js
    test262/built-ins/WeakMap/constructor.js

however if I run it with jstests.py, I can see debugger is actually launched

./js/src/tests/jstests.py obj-x86_64-pc-linux-gnu/dist/bin/js built-ins/WeakMap/constructor.js --debugger gdb -g
Excess command line arguments ignored. (/home/allstars/src/gecko/js/src/tests/test262/shell.js ...)
GNU gdb (Ubuntu 8.1-0ubuntu3.1) 8.1.0.20180409-git
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /home/allstars/src/gecko/obj-x86_64-pc-linux-gnu/dist/bin/js...done.
"/home/allstars/src/gecko/js/src/tests/shell.js" is not a core dump: File format not recognized
Loading JavaScript value pretty-printers; see js/src/gdb/README.
If they cause trouble, type: disable pretty-printer .* SpiderMonkey
SpiderMonkey unwinder is disabled by default, to enable it type:
	enable unwinder .* SpiderMonkey
warning: Missing auto-load script at offset 0 in section .debug_gdb_scripts
of file /home/allstars/src/gecko/obj-x86_64-pc-linux-gnu/dist/bin/js.
Use `info auto-load python-scripts [REGEXP]' to list them.
(gdb) 

is this also a bug in ./mach jstests ?

Thanks

Flags: needinfo?(allstars.chh)

(In reply to Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh] from comment #6)

(In reply to Edwin Takahashi (:egao, :etakahashi) from comment #5)

But that is not what's happening here; the debug flag needs to be manually set to True by passing in the --debug/-g parameter. Which means, the actual command when using mach should be:

./mach jstests --debugger ls js/src/tests/test262/built-ins/Array/prototype/forEach/15.4.4.18-7-c-ii-1.js -g
or
./mach jstests --debugger ls js/src/tests/test262/built-ins/Array/prototype/forEach/15.4.4.18-7-c-ii-1.js --debug

I was able to successfully invoke my debugger (ls) without making any changes to the code:

Cool, so jstests already accepts debugger argument, then it would be great to show this in help
./mach help jstests
because in the first place I thought ./mach jstests doesn't accept debugger argument hence filed this bug.

That's a valid concern, though I'm not sure how to add that to the help statement. ./mach jstests is just a helpful wrapper over the jstests.py and it will pass through any arguments that it does not recognize (like the -g) to jstests.py.

With your information,
now I added '-g' when running ./mach jstests, it shows some error message

./mach jstests built-ins/WeakMap/constructor.js --debugger gdb -g
Multiple tests match command line arguments, debugger can only run one
    test262/built-ins/WeakMap/constructor.js
    test262/built-ins/WeakMap/constructor.js
    test262/built-ins/WeakMap/constructor.js
    test262/built-ins/WeakMap/constructor.js

I believe that's just saying that gdb debugger can only debug through one file. If you insert a path to one specific file then that warning should go away.

Whiteboard: dev-prod-2020
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.