936279 - NS_RUNTIMEABORT puts file paths into release builds

Status: NEW

(Core :: XPCOM, defect)

Description

[(Away)]

On currently shipping builds I see over 400 file paths in xul. I believe the vast majority of these come from NS_RUNTIMEABORT. They all start with e:/builds/moz2_slave/rel-m-rel-w32_bld-000000000000/build/ (58 characters) and let's say there's an average of 50% more for the actual source path, and that adds up to 35KB.

Most of the assertion macros compile down to nothing in opt builds, but NS_RUNTIMEABORT is deliberately left enabled. Does it really need to use __FILE__ outside of debug though?

Maybe having file names is valuable enough for investigations to be worth the cost. Or maybe it was just an oversight. I don't know.

Comment 1

[Andrew McCreight [:mccr8]]

I'd think that the stack in the crash report would be enough.

Comment 2

[(Away)]

And there's probably another dozen or two KB in the abort messages, though perhaps those are more valuable.

Comment 3

[Benjamin Smedberg]

The stack in the crash report is not always reliable, especially in low-memory situations, and the abort message including the file name is sometimes quite valuable. But really all we need is the leafname, not the full path.

Comment 4

[(no longer active)]

(In reply to comment #3)
> The stack in the crash report is not always reliable, especially in low-memory
> situations, and the abort message including the file name is sometimes quite
> valuable. But really all we need is the leafname, not the full path.

Well, all that we have at our disposal is __FILE__.

Comment 5

[Trevor Saunders (:tbsaunde)]

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #4)
> (In reply to comment #3)
> > The stack in the crash report is not always reliable, especially in low-memory
> > situations, and the abort message including the file name is sometimes quite
> > valuable. But really all we need is the leafname, not the full path.
> 
> Well, all that we have at our disposal is __FILE__.

not really, we could have the build system pass the compiler -DMOZ_FILE=$(basename $CPP_FILE) and then use MOZ_FILE instead of __FILE__ in places like this.

(seems relevent to froydnj's interests so cc += him)

Comment 6

[(no longer active)]

(In reply to comment #5)
> (In reply to :Ehsan Akhgari (needinfo? me!) from comment #4)
> > (In reply to comment #3)
> > > The stack in the crash report is not always reliable, especially in low-memory
> > > situations, and the abort message including the file name is sometimes quite
> > > valuable. But really all we need is the leafname, not the full path.
> > 
> > Well, all that we have at our disposal is __FILE__.
> 
> not really, we could have the build system pass the compiler
> -DMOZ_FILE=$(basename $CPP_FILE) and then use MOZ_FILE instead of __FILE__ in
> places like this.

That will break in a number of cases, such as unfiied builds, code in headers, etc.