Add a --fix-stacks option to dmd.py

RESOLVED FIXED in mozilla35

Status

()

Core
DMD
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

unspecified
mozilla35
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
After bug 1044709 converts DMD to produce JSON output, on Linux and Mac you'll have to run the appropriate fix*.py script *and* dmd.py to get nice output, which is a pain.

My plan is to add a --fix-stacks option to dmd.py that would cause the JSON to be fixed in-place. (If the output had already been fixed, this would have no effect.)
(Assignee)

Updated

3 years ago
Blocks: 1073312
Depends on: 1044709
(Assignee)

Comment 1

3 years ago
Created attachment 8496740 [details] [diff] [review]
Add a --fix-stacks option to dmd.py
Attachment #8496740 - Flags: review?(mh+mozilla)
Comment on attachment 8496740 [details] [diff] [review]
Add a --fix-stacks option to dmd.py

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

::: memory/replace/dmd/dmd.py
@@ +173,5 @@
>  
> +# Fix stacks if necessary: first write the output to a tempfile, then replace
> +# the original file with it.
> +def maybeFixStackTraces():
> +    if args.fix_stacks:

That should be tested outside (and the function renamed not to start with maybe as a consequence)

@@ +194,5 @@
> +        # Fix stacks, writing output to a temporary file, and then overwrite
> +        # the original file.
> +        with tempfile.NamedTemporaryFile(delete=False) as tmp:
> +            for line in args.input_file:
> +                tmp.write(fixModule.fixSymbols(line))

Seems to me you intended to write tmp.write(fix(line)) but forgot.
Attachment #8496740 - Flags: review?(mh+mozilla) → review+
(Assignee)

Comment 3

3 years ago
Comment on attachment 8496740 [details] [diff] [review]
Add a --fix-stacks option to dmd.py

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

::: memory/replace/dmd/dmd.py
@@ +173,5 @@
>  
> +# Fix stacks if necessary: first write the output to a tempfile, then replace
> +# the original file with it.
> +def maybeFixStackTraces():
> +    if args.fix_stacks:

I did it that way to mirror |maybeTrimStackTraces|, which involves two options and so can't be done that way.
(Assignee)

Comment 4

3 years ago
erahm and mccr8 suggested that the JSON include a property that describes whether stacks have been fixed. It's a nice idea but there's a problem: B2G. The B2G stack fixing is done by get_about_memory.py independently of dmd.py. So it would be easy to end up with a file that claims to not have been fixed but actually has been fixed. All of which makes me nervous.
(Assignee)

Comment 5

3 years ago
Comment on attachment 8496740 [details] [diff] [review]
Add a --fix-stacks option to dmd.py

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

::: memory/replace/dmd/dmd.py
@@ +182,5 @@
> +        # XXX: should incorporate fix_stack_using_bpsyms.py here as well, like
> +        #      in testing/mochitests/runtests.py
> +        sysname = platform.system()
> +        if sysname == 'Linux':
> +            import fix_linux_stack as fixModule

I need to get fix_stack_using_bpsyms.py working in order to make the xpcshell test (bug 1073312) work on TBPL. And that requires knowing the symbolsPath. glandium, do you know how I can get the symbolsPath that in this script?

Looking around, it looks like I can get it something like this:

> options.symbolsPath = os.path.join(distdir, 'crashreporter-symbols')

and |distdir| like this:

> distdir = os.path.join(topobjdir, 'dist')

but I don't know how to get topobjdir in this script. Or maybe all that is totally heading in the wrong direction, I don't know.
(Assignee)

Updated

3 years ago
Flags: needinfo?(mh+mozilla)
(Assignee)

Comment 6

3 years ago
> I need to get fix_stack_using_bpsyms.py working in order to make the
> xpcshell test (bug 1073312) work on TBPL. And that requires knowing the
> symbolsPath.

Another possibility: the xpcshell test harness knows the symbolsPath. It already sets a couple of environment variables ($DMD, $PYTHON) to pass information to DMD's xpcshell test. It could set another one to pass the symbolsPath.
Comment 6 is probably better than munging paths in crazy ways, especially when the test environment on local builds and tbpl builds is so completely different.
Flags: needinfo?(mh+mozilla)
(Assignee)

Comment 8

3 years ago
(In reply to Nicholas Nethercote [:njn] from comment #4)
> erahm and mccr8 suggested that the JSON include a property that describes
> whether stacks have been fixed. It's a nice idea but there's a problem: B2G.
> The B2G stack fixing is done by get_about_memory.py independently of dmd.py.
> So it would be easy to end up with a file that claims to not have been fixed
> but actually has been fixed. All of which makes me nervous.

But I will make stack-fixing in dmd.py the default behaviour.
(Assignee)

Comment 9

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b73787681bd
https://hg.mozilla.org/mozilla-central/rev/2b73787681bd
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.