14989 - ramiro's stack-trace hack should be on by default

Status: RESOLVED FIXED

(Core Graveyard :: Cmd-line Features, defect, P1)

Description

[Dan Mosedale (:dmosedale, :dmose)]

Casual users of the milestone builds need to be given enough information to

encourage them to easily file useful bugs.  As such, it seems to me that M10 and

 later should by default be built either with ramiro's stack trace code turned

on
or else mozilla-apprunner.so should default to using libSegFault.so.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Turns out that libSegFault.so as shipped with RH6 doesn't seem to give useful
stack traces for multi-threaded executables.  It does, however, dump the
registers (which might conceivably be helpful in some case).  So ramiro's hack
is definitely the one that counts.

Comment 2

[chris hofmann]

shaver, whatcha goin' n do?  time to wrap m10.

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

I don't know what the effects of Ramiro's hack are on non-Linux platforms, etc.
I'm comfortable having it turned on if we think it's not a big risk for our
other Unix platforms.

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

So maybe the right thing to do is turn it on in the tip right now, and that will
help us shake out any problems in time to ship M11 with it on by default.

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Sold.

Who's going to flip the bit?

Comment 6

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

We're turning this on for M11, not M10.

Comment 7

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Let's turn it on in the default build now, and see if it kicks anyone's ass.

Leaf, can you do the honours?

Comment 8

[Daniel (Leaf) Nunes]

Sure... i take it the means of turning this on is buried somewhere in
n.p.m.unix.

Comment 9

[Daniel (Leaf) Nunes]

I am completely lame. I'll turn this on after M11 ships, and we can test it
before the alpha release. (now that i know where to look)

Comment 10

[chris hofmann]

can someone post what the hack is in this bug report.
seems like we need to get a few people to try this out
at the begining of a milestone period, not at the end.
we are at the end again..  m12 shutdown starts tomorrow..
leaf's got other stuff to do durning the milestone period...

lets get a few xheads to try the patch on a few platforms
when the hack gets posted here..

Comment 11

[Ramiro Estrugo]

the hack is broken.  some checkins broke it and its rotten now.  sorry

Comment 12

[Dan Mosedale (:dmosedale, :dmose)]

Any idea what it would take to fix it?

Comment 13

[Daniel (Leaf) Nunes]

I'm a big loser. Not going to make this in m13.

Comment 14

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Patch to make this code work again and turn it on.

Comment 15

[Dan Mosedale (:dmosedale, :dmose)]

Note that this code also has the useful function of causing mozilla to go to
sleep after a crash so that one can attach to it with a debugger.  This is very
handy on linux, since linux cannot dump useful cores for multithreaded apps.

Comment 16

[Daniel (Leaf) Nunes]

testing patch.

Comment 17

[Daniel (Leaf) Nunes]

tested on linux, seems to be working. How should i give this to reviewers@? The
patch itself that dmose has attached is tiny, but it turns on a bunch of other
stuff.

Comment 18

[Christopher Blizzard (:blizzard)]

It seems to work.  However, I don't know if I like the long 5 minute time out
and it just looks like a hang to a user.  Do we want it turned off for non-debug
builds?

Comment 19

[Dan Mosedale (:dmosedale, :dmose)]

You're right, the sleep should be turned off for non-debug builds and the
message to stderr modified to match.

Comment 20

[Dawn Endico]

leger is no longer @netscape.com
changing qa contact to default for this component

Comment 21

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

We were discussing this on IRC today... I'll take the bug and try to get an
appropriate form of the patch in...

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch to reenable in a way that might not break builds...

Comment 23

[Dan Mosedale (:dmosedale, :dmose)]

r=[e-mail]

Comment 24

[Brendan Eich [:brendan]]

Code looks fine -- if it works, sr=[e-mail]

/be

Comment 25

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'm thinking maybe this should be for DEBUG builds only.  I'm worried it:
 * might mess up talkback
 * wouldn't be too useful on builds with --enable-strip-libs

The reason I haven't checked it in yet is that it's rather intertwined with my
changes for bug 66159.

Comment 26

[Dan Mosedale (:dmosedale, :dmose)]

Part of my reasoning for wanting this is for people who download the nightlies
without talkback (in part because it seems like talkback goes through phases of
not working in one or the other of the installer or zip packaging).  But both of
these potential problems shouldn't be hard to test to see if they really cause
problems, I would think.

Comment 27

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

On my own non-debug build, which doesn't have --enable-strip-libs, the
stack traces are only barely useful, because the demangled symbols are
all NSGetModule, so the only way to get any useful information is through
addr2line, which will give the correct function names (but not line
numbers).  However, I suspect that with --enable-strip-libs, it won't
even be able to do that.  I guess I could try an --enable-strip-libs
build sometime...

Comment 28

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

OK, I've been running with this in my opt build for a few weeks now, and it's
caused some problems that make me not want to check it in for opt builds, and
perhaps not even at all.

In some cases I will see a crash and this code will go into an infinite loop
repeatedly printing the stack trace.  I'm not sure whether I've seen this in my
DEBUG build (where the timeout wasn't #ifdef-ed), but I know I've seen it in my
non-DEBUG build.

When this happens when I exit mozilla, I don't really notice.  However, since
I'm running my opt build from the gnome-panel rather than from an xterm window,
it spews this infinite loop to .xsession-errors, which fills up my /home
partition (yes, it fills 2.5GB of empty space) and causes my gnome/mozilla
profile data to get corrupted when programs attempt to write configurations back
to disk.  This has happened to me 3 or 4 times in the past week.

I'm not sure whether this can happen with the debugger-timeout enabled, but it's
*very* annoying, so I suggest that this shouldn't go in for optimized builds at
least until someone fixes the problem of infinite stack traces (for sure).

Comment 29

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Since I really need to get the stuff in bug 66159 tested on Windows and Mac,
moving this to mozilla 0.9.1

Comment 30

[Christopher Blizzard (:blizzard)]

Just some comments here.  I use this on a day to day basis in my opt builds.  It
would be nice to have a --enable-stacktrace or something that:

o Turned on the stack trace code
o Turned off the elf-dynstr stripper
o Added -g to CFLAGS and CXXFLAGS so symbols would work

Comment 31

[chris hofmann]

looks like this is something that could be move off to 0.9.2..

Comment 32

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It's part of a larger patch that I'd still like to land for 0.9.1 that involves
a bit of rearrangement of debug-only code.

Comment 33

[tenthumbs]

I don't know if this is useful but have you looked at glibc's
undocumented backtrace* functions?

Comment 34

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Pushing out again.  I'll try to land this early in 0.9.2.  Really.  (I should
have time this time around.)

Comment 35

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

FWIW, the backtrace* functions are nicely documented in /usr/include/execinfo.h

Comment 36

[tenthumbs]

I guess I'm a purist but the include file documents a particular
implementation of the functions not the functions themselves, but that's
not important here.

Comment 37

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: up-to-date patch, DEBUG only (see above)

Comment 38

[Brian Ryner (not reading)]

Comment on attachment 46617 [details] [diff] [review]
up-to-date patch, DEBUG only (see above)

r=bryner

Comment 39

[Brendan Eich [:brendan]]

Comment on attachment 46617 [details] [diff] [review]
up-to-date patch, DEBUG only (see above)

sr=[e-mail] -- what's not to love?

/be

Comment 40

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

All fixes except for turning it on checked in.

Comment 41

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

->0.9.8, since I haven't sent out newsgroup posts about this yet.  (Somebody
remind me in two weeks or something so I don't notice this at the very end of
every milestone. :-)

Comment 42

[Andreas Franke (gone)]

Reminding...

Comment 43

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Remainder of patch checked in to trunk, 2002-07-04 22:31 PDT.

Comment 44

[Aleksey Nogin]

Can something like this be used to fix bug 148453 on non-debug builds?

Comment 45

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So, does anyone else think anything else should be done for this bug, or should
I close it?  (Perhaps other things to do should be filed as separate bugs?)

I'm against turning it on for opt builds based on the experiences I've had with
it filling up my .xsession-errors until my disk was full, and also since the opt
stacks aren't too useful because of the way we strip symbols out of components.

Comment 46

[Dan Mosedale (:dmosedale, :dmose)]

Closing this sounds fine.  Other improvements can be filed as new bugs.  Thanks
for doing this.

Comment 47

[ramiro]

Yanking myself from cc.

Comment 48

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

OK, fixed then.