With '-g' in jit-tests.py give a list of options to debug

RESOLVED FIXED in Firefox 48

Status

()

Core
JavaScript Engine
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: h4writer, Assigned: h4writer)

Tracking

unspecified
mozilla48
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
Something that I often do is narrowing down the list of matches by providing the name of the test and trying to debug that using '-g'. With '--ion' that often gives multiple tests and as a result it doesn't want to execute gdb on the desired testcase.

I added some logic to jit-tests to give a list of possible matches and to select which one to run under gdb or debugger. (Next to that I also listed the flags of the testcases. To make it easier to select the right one).
(Assignee)

Comment 1

2 years ago
Created attachment 8731781 [details] [diff] [review]
Patch
Assignee: nobody → hv1989
Attachment #8731781 - Flags: review?(sphink)
Comment on attachment 8731781 [details] [diff] [review]
Patch

Review of attachment 8731781 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit-test/jit_test.py
@@ +277,5 @@
>              print('Multiple tests match command line'
>                    ' arguments, debugger can only run one')
> +            jobs = list(job_list)
> +            for i in range(len(jobs)):
> +                tc = jobs[i]

for i, tc in enumerate(jobs, 1):

@@ +281,5 @@
> +                tc = jobs[i]
> +                flags = ""
> +                if len(tc.jitflags) != 0:
> +                    flags = "({})".format(' '.join(tc.jitflags))
> +                print('{}) {} {}'.format(i + 1, tc.path, flags))

you can lose the + 1 by passing 1 to enumerate(jobs, 1) above.

@@ +297,5 @@
> +            except ValueError:
> +                print('Unrecognized input')
> +                sys.exit(1)
> +
> +            tc = jobs[item - 1]

I would prefer this to be factored out into something like

def display_job(j):
    flags = ""
    ...
try:
    tc = choose_item(jobs, max_items=50, display=display_job)
except Exception as e:
    sys.exit(str(e) + '\n')

and then raise Exception('Input is not between 1 and bajillion') in choose_item. (The display_job def can be local.) This seems like enough code to me to clutter things up a bit, not that the rest of the method is any better.
Attachment #8731781 - Flags: review?(sphink) → review+
(Assignee)

Comment 3

2 years ago
Created attachment 8732810 [details] [diff] [review]
Patch

Good idea. Since most code has been changed, re-requesting r?
Attachment #8731781 - Attachment is obsolete: true
Attachment #8732810 - Flags: review?(sphink)
Attachment #8732810 - Flags: review?(sphink) → review+

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d07c014f69c1
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.