149032 - [BeOS: C++Exceptions] OOM Crash dereferencing null this [@nsFrame::nsFrame]

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect, P2)

Description

[timeless]

segment violation occurred
nsFrame::nsFrame(void):
__7nsFrame:
+0019  ed7f4711:   *          0289    movl    %eax, (%edx)
mozilla-bin:sc
   frame         retaddr
fd000b40   ed7f25ba  nsContainerFrame::nsContainerFrame(void) + 0000001a
fd000b54   ed7e5262  nsBlockFrame::nsBlockFrame(void) + 0000001a
fd000b68   ed7e5221  NS_NewBlockFrame(nsIPresShell *, nsIFrame **, unsigned 
int) + 00000035
fd000b84   ed873d44  nsTableCreator::CreateTableCellInnerFrame(nsIFrame **) + 
00000024
fd000b9c   ed877b95  
nsCSSFrameConstructor::ConstructTableCellFrame(nsIPresShell *, nsIPresContext *
, nsFrameConstructorState &, nsIContent *, nsIFrame *, nsIStyleContext *, 
nsTableCreator &, int, nsFrameItems &, nsIFrame *&, nsIFrame *&, int &) + 
00000151
fd000bf4   ed878719  nsCSSFrameConstructor::TableProcessChild(nsIPresShell *, 
nsIPresContext *, nsFrameConstructorState &, nsIContent *, nsIContent *, 
nsIFrame *, nsIAtom *, nsIStyleContext *, nsTableCreator &, nsFrameItems &, 
nsIFrame *&) + 00000311
fd000c64   ed878314  nsCSSFrameConstructor::TableProcessChildren(nsIPresShell *
, nsIPresContext *, nsFrameConstructorState &, nsIContent *, nsIFrame *, 
nsTableCreator &, nsFrameItems &, nsIFrame *&) + 00000310
fd000d84   ed877623  nsCSSFrameConstructor::ConstructTableRowFrame(nsIPresShell 
*, nsIPresContext *, nsFrameConstructorState &, nsIContent *, nsIFrame *, 
nsIStyleContext *, nsTableCreator &, int, nsFrameItems &, nsIFrame *&, int &) +
 00000167
fd000dd4   ed87f8cc  
nsCSSFrameConstructor::ConstructFrameByDisplayType(nsIPresShell *, 
nsIPresContext *, nsFrameConstructorState &, nsStyleDisplay const *, nsIContent 
*, nsIFrame *, nsIStyleContext *, nsFrameItems &) + 000011ec
fd000f88   ed880b7e  nsCSSFrameConstructor::ConstructFrameInternal(nsIPresShell 
*, nsIPresContext *, nsFrameConstructorState &, nsIContent *, nsIFrame *, 
nsIAtom *, int, nsIStyleContext *, nsFrameItems &, int) + 0000041a
fd000fe8   ed880708  nsCSSFrameConstructor::ConstructFrame(nsIPresShell *, 
nsIPresContext *, nsFrameConstructorState &, nsIContent *, nsIFrame *, 
nsFrameItems &) + 00000154
fd00103c   ed88394d  nsCSSFrameConstructor::ContentAppended(nsIPresContext *, 
nsIContent *, int) + 00001109
fd001318   ed455772  StyleSetImpl::ContentAppended(nsIPresContext *, nsIContent 
*, int) + 00000032
fd00133c   ed831b4f  PresShell::ContentAppended(nsIDocument *, nsIContent *, 
int) + 0000004f
fd001360   ed3fb90b  nsDocument::ContentAppended(nsIContent *, int) + 0000009b
fd001388   ed3012b9  nsHTMLDocument::ContentAppended(nsIContent *, int) + 
000000ad
fd0013b8   ed2fa42f  HTMLContentSink::NotifyAppend(nsIContent *, int) + 
00000037
fd0013d8   ed2f2c7b  SinkContext::FlushTags(int) + 0000018f
fd001404   ed2f1c7f  SinkContext::DidAddContent(nsIContent *, int) + 000000e7
fd001424   ed2f21d2  SinkContext::CloseContainer(nsIParserNode const &) + 
0000015a
fd001464   ed2f5b47  HTMLContentSink::CloseContainer(nsIParserNode const &) + 
0000003b
fd001480   ed69be57  CNavDTD::CloseContainer(nsCParserNode const *, nsHTMLTag, 
int) + 0000023f
fd0014a0   ed69bf06  CNavDTD::CloseContainersTo(int, nsHTMLTag, int) + 00000092
fd0014dc   ed6985c8  CNavDTD::HandleDefaultStartToken(CToken *, nsHTMLTag, 
nsCParserNode *) + 00000270
fd001520   ed699326  CNavDTD::HandleStartToken(CToken *) + 000002b2
fd001558   ed697cbd  CNavDTD::HandleToken(CToken *, nsIParser *) + 000006ad
fd0015d8   ed696e56  CNavDTD::BuildModel(nsIParser *, nsITokenizer *, 
nsITokenObserver *, nsIContentSink *) + 000001fa
fd00160c   ed6a98e3  nsParser::BuildModel(void) + 000000a3
fd00163c   ed6a965b  nsParser::ResumeParse(int, int, int) + 000000ff
fd001664   ed6ab22a  nsParser::OnDataAvailable(nsIRequest *, nsISupports *, 
nsIInputStream *, unsigned int, unsigned int) + 00000112
fd0016a0   ece1f3da  nsDocumentOpenInfo::OnDataAvailable(nsIRequest *, 
nsISupports *, nsIInputStream *, unsigned int, unsigned int) + 0000003e
fd0016c4   ecbeb47c  nsMultiMixedConv::SendData(char *, unsigned int) + 
00000164
fd00170c   ecbea9cb  nsMultiMixedConv::OnDataAvailable(nsIRequest *, 
nsISupports *, nsIInputStream *, unsigned int, unsigned int) + 0000045b
fd001764   ece1f3da  nsDocumentOpenInfo::OnDataAvailable(nsIRequest *, 
nsISupports *, nsIInputStream *, unsigned int, unsigned int) + 0000003e
fd001788   ecbdf79c  nsStreamListenerTee::OnDataAvailable(nsIRequest *, 
nsISupports *, nsIInputStream *, unsigned int, unsigned int) + 0000015c
fd0017c8   ecc16d76  nsHttpChannel::OnDataAvailable(nsIRequest *, nsISupports *
, nsIInputStream *, unsigned int, unsigned int) + 000000e6
fd0017f4   ecbdece4  nsOnDataAvailableEvent::HandleEvent(void) + 000000a8
fd00182c   ecbce12e  nsARequestObserverEvent::HandlePLEvent(PLEvent *) + 
00000026
fd00183c   eca723c7  PL_HandleEvent + 0000001f
fd001854   eca722e3  PL_ProcessPendingEvents + 00000077
fd00186c   eca73109  nsEventQueueImpl::ProcessPendingEvents(void) + 00000041
fd001894   eccf5bd5  nsAppShell::Run(void) + 0000010d
fd0018b8   eccb8820  nsAppShellService::Run(void) + 00000024
fd0018c8   8000fee6  main1(int, char **, nsISupports *) + 000009ba
fd001988   8001045d  main + 0000011d
fd0019b4   8000b275  _start + 00000061
mozilla-bin:registers
registers
        ^ Ambiguous symbol
mozilla-bin:regs
 eax eda3cf40   ebp fd000b40   cs 001b
 edx 00000000   esi 00000000   ss 0023
 ecx 00000000   edi fd000dc4   ds 0023
 ebx eda6387c   esp fd000b3c   es 0023
                               fs 71e3
 eflags 00010282  eip ed7f4711
 trap_no 0000000e  error_code 00000006
mozilla-bin:il
nsFrame::nsFrame(void):
__7nsFrame:
+0019  ed7f4711:   *          0289    movl    %eax, (%edx)
+001b  ed7f4713:      00000690838b    movl    0x00000690(%ebx), %eax
+0021  ed7f4719:              0289    movl    %eax, (%edx)
+0023  ed7f471b:            04428d    leal    0x00000004(%edx), %eax
+0026  ed7f471e:    000000000442c7    movl    $0x00000000, 0x00000004(%edx)

BeOS desktop 5.0 1000009 BePC unknown
[BeOS5.0.3PE]
Mozilla 2002060103 [nightly from ftp server]
Mozilla was using 200044KB of memory (process controller)
BeOS system resources and caches are using ~450MB of ram (process controller)
[this second number is a moving target since it's live, whereas the first 
number shouldn't and hasn't changed because mozilla is crashed on its main 
thread]
System has 512MB of Ram and No VM.

By my calculations BeOS shouldn't have given BeZilla a malloc failure, but it's 
possible that it decided to anyway.

Comment 1

[David Bradley]

Is the assembler posted from the start of nsFrame::nsFrame, or where exactly
does this assembler reside? Just hard to know what edx represents without some
context.

Comment 2

[timeless]

note to self AT&T style confuses people, this is intel style from bdb:
+0000  ed7f46f8:        push    ebp
+0001  ed7f46f9:        mov     ebp, esp
+0003  ed7f46fb:        push    ebx
+0004  ed7f46fc:        call    0ed7f4701h <nsFrame::nsFrame(void)+000000009h>
+0009  ed7f4701:        pop     ebx
+000a  ed7f4702:        add     ebx, 00026f17bh
+0010  ed7f4708:        mov     edx, [ebp+000000008h]
+0013  ed7f470b:        mov     eax, [ebx+00000061ch]
+0019  ed7f4711:        mov     [edx], eax
+001b  ed7f4713:        mov     eax, [ebx+000000690h]
+0021  ed7f4719:        mov     [edx], eax
+0023  ed7f471b:        lea     eax, [edx+000000004h]
+0026  ed7f471e:        mov     [edx+000000004h], 000000000h
+002d  ed7f4725:        mov     [eax+000000004h], 000000000h
+0034  ed7f472c:        mov     [eax+000000008h], 000000000h
+003b  ed7f4733:        mov     [eax+00000000ch], 000000000h
+0042  ed7f473a:        mov     eax, [ebx+000000694h]
+0048  ed7f4740:        mov     [edx], eax
+004a  ed7f4742:        mov     [edx+000000024h], 000000406h
+0051  ed7f4749:        mov     eax, edx
+0053  ed7f474b:        pop     ebx
+0054  ed7f474c:        mov     esp, ebp
+0056  ed7f474e:        pop     ebp
+0057  ed7f474f:        retn    

Anyway, after talking w/ dbradley we've concluded that this is null.

so walking up the callstack we get to:

+0000  ed7e51ec:        push    ebp
+0001  ed7e51ed:        mov     ebp, esp
+0003  ed7e51ef:        push    esi
+0004  ed7e51f0:        push    ebx
+0005  ed7e51f1:        call    0ed7e51f6h <NS_NewBlockFrame(nsIPresShell *,
+00000000ah>
+000a  ed7e51f6:        pop     ebx
+000b  ed7e51f7:        add     ebx, 00027e686h
+0011  ed7e51fd:        mov     esi, [ebp+00000000ch]
+0014  ed7e5200:        test    esi, esi
+0016  ed7e5202:        jnz     0ed7e5210h <NS_NewBlockFrame(nsIPresShell *,$+
000000024h>
+0018  ed7e5204:        mov     eax, 080004003h
+001d  ed7e5209:        jmp     0ed7e523dh <NS_NewBlockFrame(nsIPresShell *,Q+
000000051h>
+001f  ed7e520b:        nop     
+0020  ed7e520c:        lea     esi, [esi]
+0024  ed7e5210:        mov     eax, [ebp+000000008h]
+0027  ed7e5213:        push    eax
+0028  ed7e5214:        push    04ch
+002a  ed7e5216:        call    0ed7f4690h <nsFrame::operator new(unsigned l>
+002f  ed7e521b:        push    eax
+0030  ed7e521c:        call    0ed7e5248h <nsBlockFrame::nsBlockFrame(void)>

And then we compared this to MSVC's output for something:
027E55E2   call        nsFrame::operator new (02691230)
027E55E7   add         esp,8
027E55EA   mov         dword ptr [ebp-8],eax
027E55ED   cmp         dword ptr [ebp-8],0
027E55F1   je          NS_NewViewportFrame+60h (027e5600)
027E55F3   mov         ecx,dword ptr [ebp-8]
027E55F6   call        ViewportFrame::ViewportFrame (027e5630)

The key here is that MSVC generated code {027E55ED} that checked for a null 
pointer. Whereas {+001d} did not. I think BeOS requires C++ Exceptions.

Comment 3

[David Bradley]

The short of it is, that any operator new functions other than placement new
functions, ideally should throw an exception if exceptions are present.

This probably isn't a big deal, given that none of the Mozilla codebase is not
exception aware. The end result will be the same. If new doesn't throw an
exception we get an access violation when a constructor tries to access the
object. If new were to throw an exception, the program will terminate with an
unhandled exception. It would be nice if operator new could issue some
diagnostic when allocation fails.

Comment 4

[timeless]

Attachment: code to generate later ASM

Comment 5

[timeless]

Attachment: ASM generated from C++

Comment 6

[timeless]

the fix is to tag all of our XP new functions that return null (which should be
all of them, because the portability guidelines insist that we not use
exceptions) with throw().  So the question is how to do that.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

os->all, since this happens on linux as well (at least the assembler generated
by my gcc doesn't show a null check after A::operator new in timeless's testcase)

Comment 8

[hacker formerly known as [e-mail]]

timeless: as a stopgap, do you still see the problem if you compile with
-fcheck-new ?

Comment 9

[timeless]

I checked the codegen and that makes both check, so sure we can use that as a 
stop gap.

Comment 10

[hacker formerly known as [e-mail]]

Taking.

Comment 11

[hacker formerly known as [e-mail]]

Attachment: Sprinkle CPP_THROW_NEW liberally

Define CPP_THROW_NEW ...
as throw() in config.mak
as empty in config/mac/MacDefines.h
as throw() in configure.in by default unless --enable-cpp-exceptions is given.

For some reason, my CW build didn't like it when I defined CPP_THROW_NEW as
throw() .  It kept complaining about an exceptions declaration mismatch.  If I
added CPP_THROW_NEW to the function declaration as well, then CW complained
about the spots where new was actually used.

Comment 12

[Scott Collins]

Comment on attachment 89908 [details] [diff] [review]
Sprinkle CPP_THROW_NEW liberally

sr=scc.  How unfortunate that we have so many implementations of |new|, and for
the most part, for very wrong reasons, like clearing memory

Comment 13

[hacker formerly known as [e-mail]]

Attachment: Use compile test to determine if throw() is acceptible

This patch replaces most of the hardcoded compiler checks with a generic
compile time test. There are still exceptions for msvc & vacpp.  The test is
only attempted if we are building without exceptions enabled (the default).

Comment 14

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

Comment on attachment 90661 [details] [diff] [review]
Use compile test to determine if throw() is acceptible

It seems like it might be a little clearer to put the known results at the top,
e.g.:

if test -n "$_MOZ_CPP_EXCEPTIONS" -o -n "$VACPP"; then
    _use_cpp_empty_exception_handler=no
elif test -n "$_WIN32_MSVC"; then
    _use_cpp_empty_exception_handler=yes
else
    <...>
endif

Other than that (and it's ok with me if you don't want to make this change
since you've tested this version a bit), r=dbaron.

Comment 15

[Mike Kaply [:mkaply]]

The OS/2 build was DOA on July 4 at 8AM. The July 3 AM build works OK.

The first issue was that we were getting an internal compiler error 
with the CPP_THROW_NEWs, so I changed configure.in to define 
CPP_THROW_NEW to nothing for VACPP. 

I believe this is causing a problem for some reason.

I have some more investigation to do, but I was hoping someone could 
shed some light on this.

If I define CPP_THROW_NEW to nothing, is there ANYTHING that is 
different between a July 3 build and a July 4 build?

July 3 obviously had CPP_THROW_NEW in for VACPP I believe...

Thanks for any help.

Comment 16

[hacker formerly known as [e-mail]]

If CPP_THROW_NEW is defined to nothing, then there shouldn't be any difference
between the builds wrt this bug.  I'm confused as to why setting the define to
empty would cause a problem.  Are the builds still DOA with the workaround that
you checked in yesterday?

The July 3 8AM build would have had only the original patch applied which only
added the CPP_THROW_NEW define to the implementations of |operator new|. 
CPP_THROW_NEW was only being defined to |throw()| when using gcc. The changes on
July 3 added CPP_THROW_NEW to the |operator new| declarations as well and
removed the gcc-only workaround.  

Comment 17

[Mike Kaply [:mkaply]]

I don't fully understand it either, but I am not convinced it is this bug, I
just wanted some advice.

We crash doing pretty much anything, but aaronl's checkins are also suspect
because they involve menus which are also in the callstack.

Note the actual error is a privileged instruction exception.

 0x00B8AC50                                                                    
                                
 NS_AllocateContiguousHandleWithData(const nsSharedBufferHandle<unsigned
short>*,unsigned int,const nsAString*)
 NS_AllocateContiguousHandleWithData(const nsSharedBufferHandle<unsigned
short>*,const nsAString&,unsigned int)
 nsSharableString::do_AssignFromReadable(const nsAString&)                     
                                
 nsAString::Assign(const nsAString&)                                           
                                
 nsSharableString::operator=(const nsAString&)                                 
                                
 nsPromiseFlatString::nsPromiseFlatString(const nsAString&)                    
                                
 PromiseFlatString(const nsAString&)                                           
                                
 GetAtomHashEntry(const nsAString&)                                            
                                
 NS_NewAtom(const nsAString&)                                                  
                                
 nsDOMEvent::SetEventType(const nsAString&)                                    
                                
 nsDOMEvent::InitEvent(const nsAString&,int,int)                               
                                
 nsBoxFrame::FireDOMEvent(nsIPresContext*,const nsAString&)                    
                                
 nsMenuBarFrame::SetActive(int)                                                
                                
 nsMenuBarFrame::~nsMenuBarFrame()                                             
                                
 nsFrame::Destroy(nsIPresContext*)                                             
                                
 nsContainerFrame::Destroy(nsIPresContext*)                                    
                                
 nsBoxFrame::Destroy(nsIPresContext*)                                          
                                
 nsMenuBarFrame::Destroy(nsIPresContext*)                                      
                                
 nsFrameList::DestroyFrames(nsIPresContext*)                                   
                                
 nsContainerFrame::Destroy(nsIPresContext*)                                    
                                
 nsBoxFrame::Destroy(nsIPresContext*)                                          
                                
 nsFrameList::DestroyFrames(nsIPresContext*)                                   
                                
 nsContainerFrame::Destroy(nsIPresContext*)                                    
                                
 nsBoxFrame::Destroy(nsIPresContext*)                                          
                                
 nsFrameList::DestroyFrames(nsIPresContext*)                                   
                                
 nsContainerFrame::Destroy(nsIPresContext*)                                    
                                
 nsBoxFrame::Destroy(nsIPresContext*)                                          
                                
 nsFrameList::DestroyFrames(nsIPresContext*)                                   
                                
 nsContainerFrame::Destroy(nsIPresContext*)                                    
                                
 nsBoxFrame::Destroy(nsIPresContext*)                                          
                                
 nsFrameList::DestroyFrames(nsIPresContext*)                                   
                                
 nsContainerFrame::Destroy(nsIPresContext*)                                    
                                
 ViewportFrame::Destroy(nsIPresContext*)                                       
                                
 FrameManager::Destroy()                                                       
                                
 PresShell::Destroy()                                                          
                                
 DocumentViewerImpl::Destroy()                                                 
                                
 nsDocShell::Destroy()                                                         
                                
 nsWebShell::Destroy()                                                         
                                
 nsXULWindow::Destroy()                                                        
                                
 nsWebShellWindow::Destroy()                                                   
                                
 nsWebShellWindow::Close()                                                     
                                
 nsWebShellWindow::HandleEvent(nsGUIEvent*)                                    
                                
 nsWindow::DispatchEvent(nsGUIEvent*,nsEventStatus&)                           
                                
 nsWindow::DispatchWindowEvent(nsGUIEvent*)                                    
                                
 nsWindow::DispatchStandardEvent(unsigned int,unsigned char)                   
                                
 nsWindow::ProcessMessage(unsigned long,void*,void*,void*&)                    
                                
 fnwpNSWindow                                                                  
                                
 0x1E935800                                                                    
                                
 nsAppShell::Run()                                                             
                                
 nsAppShellService::Run()                                                      
                                
 main1(int,char**,nsISupports*)                                                
                                
 main                                                                          
                                
 _start                                                                        
                                
 0x1C04C183                                                                    
                                

Comment 18

[hacker formerly known as [e-mail]]

Attachment: Updated with dbaron's suggestions

Comment 19

[hacker formerly known as [e-mail]]

Comment on attachment 90868 [details] [diff] [review]
Updated with dbaron's suggestions

Of course, I managed to screw that up.

Comment 20

[hacker formerly known as [e-mail]]

*** Bug 156794 has been marked as a duplicate of this bug. ***

Comment 21

[Colin Blake]

(continuing the discussion from bug 156794)

No, unfortunately the configure test works but the real code fails. That's why
we can't use the suggested new test.

Comment 22

[hacker formerly known as [e-mail]]

Which means that the test isn't good enough. :-/  The problem is that our
compile flags are spread out over several variables for various reasons, so
we're not testing the exact combination of flags that we actually build with.  I
suspect that if we even just threw in the MOZ_OPTIMIZE_FLAGS that we would catch
that both OpenVMS' & OS/2's bustage. *sigh*  

Comment 23

[hacker formerly known as [e-mail]]

I'm obviously not getting back to this anytime soon.  Looks like we're stuck
with the hardcoded valus.