Make fix-linux-stack.pl and fix_macosx_stack.py work with DMD

RESOLVED FIXED in Firefox 19

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

Trunk
mozilla20
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox19 fixed, firefox20 fixed, b2g18 fixed)

Details

Attachments

(4 attachments)

(Assignee)

Description

6 years ago
Created attachment 690261 [details] [diff] [review]
draft patch

New DMD (bug 717853) relies on fix-linux-stack.pl and fix_macosx_stack.py.  It needs to process lines that look like this:

   0x7ffd2202c275 ???[dmdd64/dist/lib/libxul.so +0xC47275]

Currently, both scripts don't allow an address before the symbol/library/offset trio.

The attached patch fixes this, but I'm not sure if I've broken anything else that relies on these scripts.

Also, fix_macosx_stack.py's underscore stripping is completely bogus, AFAICT -- it causes me to get names like |eplace_init| instead of |replace_init|.  Those symbols don't seem to have leading underscores.  I'm not sure what's going on here.
Attachment #690261 - Flags: feedback?(dbaron)
(Assignee)

Updated

6 years ago
Summary: Make fix-linux-stack.pl and fix_macosx_stack.py more flexible → Make fix-linux-stack.pl and fix_macosx_stack.py work with DMD
(Assignee)

Updated

6 years ago
Attachment #690261 - Flags: feedback?(dbaron)
(Assignee)

Comment 1

6 years ago
Created attachment 690695 [details] [diff] [review]
(part 1) - DMD: Print PCs at the end of lines.

An easier option it to modify DMD's output to conform to what
fix-linux-stack.pl and fix_macosx_stack.py already handle.  This patch moves
the PC to the end of the line.  This is arguably a better place for it since
it's usually the least interesting piece of info on a line.

jlebar, you might need to modify your b2g fixup script to handle this.
Attachment #690695 - Flags: review?(justin.lebar+bug)
(Assignee)

Updated

6 years ago
Assignee: nobody → n.nethercote
Do you have sample output kicking around so I don't have to rebuild?
(Assignee)

Comment 3

6 years ago
Created attachment 690719 [details] [diff] [review]
(part 2) - Remove fix-macosx-stack.pl because it's unused.

MXR confirms it, fix-macosx-stack.pl is dead.  (And it has been since bug
539516 landed, over 2.5 years ago).
Attachment #690719 - Flags: review?(ted)
(Assignee)

Comment 4

6 years ago
Created attachment 690725 [details] [diff] [review]
(part 3) - Fix fix_macosx_stack.py.

fix_macosx_stack.py is busted.  Here's what it does.

- The script runs file+address pairs through 'atos', which (a) gets the
  symbol name, (b) strips the leading underscore (always present on Mac) and
  (c) demangles C++ symbols (except for a few cases where it fails, not sure
  why;  maybe a bug in its demangler).

- Then, for results which don't contain whitespace (and note that many C++
  names *do* contain whitespace in their arguments list) we strip the first
  char (in theory an underscore, though usually not) and then run the result
  through c++filt.  This is always a no-op thanks to the unneeded first-char
  stripping which invalidates valid mangled names!

- For results that do contain whitespace, the atos_sym_re regexp fails and so
  we fail to invert the order of the fileline and library, which isn't a big
  deal but does make the output of different entries slightly inconsistent.

It appears that the author of the fix_macosx_stack.py thought that atos did
(a) but not (b) and (c).  Or, maybe atos didn't use to do that, but now it
does (on my 10.7 machine, at least, it does)... but the whitespace snafu in
the 2nd bullet point suggests otherwise.

Anyway, the c++filt call is pointless... except that it has a better (less
buggy?) demangler than atos, and for the few cases where atos fails to
demangle, c++filt succeeds... so long as we don't bogusly strip the first
char.

Overall, despite this bustedness, fix_macosx_stack.py produces mostly ok
stacks -- even the mishandled entries are still readable.  But if you've ever
seen Mac stack traces with some names that are (a) missing the first char, or
(b) still mangled, this is why.

This patch fixes the problems.

- Changes atos_sym_re so it matches names containing whitespace.

- Only calls cxxfilt() on still-mangled names, which start with "_Z".

- Uses "name" instead of "symbol" to refer to what's produced by atos, because
  it's no longer a vanilla symbol, being potentially underscore-stripped and
  demangled.
Attachment #690725 - Flags: review?(ted)
(Assignee)

Comment 5

6 years ago
(In reply to Justin Lebar [:jlebar] from comment #2)
> Do you have sample output kicking around so I don't have to rebuild?

Unreported: 11 blocks in block group 1 of 1,993
 360,448 bytes (360,448 requested / 0 slop)
 0.50% of the heap (0.50% cumulative);  2.03% of unreported (2.03% cumulative)
 Allocated at
   malloc (/home/njn/moz/mi2/memory/build/replace_malloc.c:151) 0x417170
   js_malloc(unsigned long) (/home/njn/moz/mi2/dmdd64/js/src/./../../dist/include/js/Utility.h:148) 0x7f67acd341a1
   js::detail::BumpChunk::new_(unsigned long) (/home/njn/moz/mi2/js/src/ds/LifoAlloc.cpp:19) 0x7f67acd3401a
   js::LifoAlloc::getOrCreateChunk(unsigned long) (/home/njn/moz/mi2/js/src/ds/LifoAlloc.cpp:94) 0x7f67acd34458
   js::LifoAlloc::alloc(unsigned long) (/home/njn/moz/mi2/js/src/ds/LifoAlloc.h:216) 0x7f67ac9edf51
   js::analyze::Bytecode* js::LifoAlloc::new_<js::analyze::Bytecode>() (/home/njn/moz/mi2/js/src/ds/LifoAlloc.h:343) 0x7f67acfe810f
   js::analyze::ScriptAnalysis::analyzeBytecode(JSContext*) (/home/njn/moz/mi2/js/src/jsanalyze.cpp:616) 0x7f67acfe76a9
   JSScript::makeAnalysis(JSContext*) (/home/njn/moz/mi2/js/src/jsinfer.cpp:5551) 0x7f67aca92487


Is that enough, or do you want a whole file?
Attachment #690719 - Flags: review?(ted) → review+
> Is that enough, or do you want a whole file?

That's plenty, but I wanted something to test fix_b2g_stack.py on, so something which hadn't been fixed.  I'll just rebuild with your patch, not a big deal.
Comment on attachment 690695 [details] [diff] [review]
(part 1) - DMD: Print PCs at the end of lines.

r=me, and I've updated fix_b2g_stack.py in my branch.  I can merge that change once this gets committed.

https://github.com/jlebar/B2G/commits/newdmd
Attachment #690695 - Flags: review?(justin.lebar+bug) → review+
Comment on attachment 690695 [details] [diff] [review]
(part 1) - DMD: Print PCs at the end of lines.

[Triage Comment]
DMD is npotb.
Attachment #690695 - Flags: approval-mozilla-b2g18+
Attachment #690695 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 11

6 years ago
ted: review ping on patch 3.
Comment on attachment 690725 [details] [diff] [review]
(part 3) - Fix fix_macosx_stack.py.

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

Sorry for the delay, this looks good.

::: tools/rb/fix_macosx_stack.py
@@ +124,5 @@
>                  # Print the first two forms as-is, and transform the third
> +                (name, library, fileline) = name_result.groups()
> +                # atos demangles, but occasionally it fails.  cxxfilt can mop
> +                # up the remaining cases(!), which will begin with '_Z'.
> +                if (name[0:2] == '_Z'):

name.startswith("_Z")
Attachment #690725 - Flags: review?(ted) → review+
(Assignee)

Comment 13

6 years ago
Part 3:

https://hg.mozilla.org/integration/mozilla-inbound/rev/2b374ff0f529

Thanks, Ted!
Whiteboard: [leave open]
https://hg.mozilla.org/mozilla-central/rev/2b374ff0f529
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Part 3:
https://hg.mozilla.org/releases/mozilla-aurora/rev/53d60295131d
https://hg.mozilla.org/releases/mozilla-b2g18/rev/08d0867c736c
status-b2g18: --- → fixed
status-firefox19: --- → fixed
status-firefox20: --- → fixed
(Assignee)

Updated

6 years ago
Component: General → DMD
You need to log in before you can comment on or make changes to this bug.