62996 - Linux speculative RTTI heuristics break in gcc-2.95.2

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P3)

Description

[Chris Waterson]

Urgh. Although the vtable layout is the same, the code generated for the RTTI 
stub has changed between egcs-1.1.2 and gcc-2.95.2. I spent half an hour trying 
to figure out how to update the heuristics, but the function prologue is now...

  push %ebp
  mov  %esp,%ebp
  sub  <some constant>,$esp

I've seen values of $0x8 and $0x14 as the constant; there may be others. (I'm 
not sure why it'd change...multiple inheritance?)

Anyway, filtering on this prologue is too weak: we end up calling a *lot* of 
routines, wreaking havoc along the way, and eventually aborting when we trollop 
through some static NSPR callback.

Need to fix this when we upgrade...

Comment 1

[Chris Waterson]

We'll need to fix this some day.

Comment 2

[Roger Chickering]

I'm using gcc 2.96, and the typeinfo code that's checked in works.
While I was waiting for the build to complete, though, I implemented 
a different approach that also works and will probably work with 2.95 
too; I'll attach a patch.

-Roger

Comment 3

[Roger Chickering]

Attachment: Different approach to deciding whether to use RTTI in typeinfo.cpp

Comment 4

[Roger Chickering]

Ack!! I'm really spazzing tonight.  Ignore that patch; I hacked
the wrong file.  Putting those changes in the right place causes
mozilla to crash.

Comment 5

[Roger Chickering]

Attachment: Fix RTTI problem w/ gcc 2.96, improved inferred types

Comment 6

[Roger Chickering]

I found a heuristic that works for gcc 2.96 on all the cases
that I thought it should.  As you probably know, that heuristic
only works for classes whose base class has no data members, because
the heuristic assumes the vtable is at the beginning of the object
and the compiler puts it at the end of the base class part of
the object.

I also modified infer_type() in TraceMalloc.pm to be able to match
stacks which are prefixes of other stacks in types.dat.  I thought
this was a good change because we can classify things as specifically
as we want while still falling back to less specific substacks.

I also updated types.dat.  As you say this is sweaty work!! :-)

waterson: if these changes look good will you check them in for
me please?  Still no CVS commit access.

Thanks!
-Roger

Comment 7

[Chris Waterson]

[s]r=waterson, looks great. Shaver, sr= this patch, and sr= bug 87698, too.

Comment 8

[Chris Waterson]

Oops. Forgot to cc shaver. blizzard, maybe you could r= this, too.

Comment 9

[Roger Chickering]

I still need r= before I can check in my fix.  shaver?  blizzard?
The changes to nsTypeInfo.cpp and TraceMalloc.pm are very small, and
none of this stuff goes into the real buld.

Thanks!
-Roger

Comment 10

[Christopher Blizzard (:blizzard)]

r=blizzard

Comment 11

[Roger Chickering]

I checked in nsTypeInfo.cpp.  After I get permission to check in
changes under tools, I'll check in the rest of the patch and close this
bug.

Comment 12

[Roger Chickering]

Checked in tools/trace-malloc changes.

Comment 13

[Brian Crowder]

Is this bug fixed?