Closed Bug 1197650 Opened 9 years ago Closed 9 years ago

crash in gfxFontGroup::FindNonItalicFaceForChar

Categories

(Core :: Graphics: Text, defect)

Product:
Core
Component:
Graphics: Text
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla43
Tracking Flags:
Tracking Status
firefox41 --- fixed
firefox42 --- fixed
firefox43 --- fixed

People

(Reporter: jtd, Assigned: jtd)

References

Details

Crash Data

Attachments

(1 file, 1 obsolete file)

patch, skip duplicate call to Valid()
978 bytes, patch
m_kato
: review+
ritu
: approval-mozilla-aurora+
ritu
: approval-mozilla-beta+
Details | Diff | Splinter Review 
Searching through gfx-related top crashers for 41.b builds, #9 is a null-deref within gfxFontGroup::FindNonItalicFaceForChar. The problem here is that the Valid() check already occurs within FindOrMakeFont. If !Valid() a nullptr is returned. So the code here just needs to do a null-check rather than calling Valid() again.

https://crash-stats.mozilla.com/report/index/bf1c51b5-5bcc-4bb8-be6f-62e1a2150818
Summary: crash in → crash in gfxFontGroup::FindNonItalicFaceForChar
Crash Signature: gfxFontGroup::FindNonItalicFaceForChar(gfxFontFamily*, unsigned int)

        Attachment #8651568 -
        Flags: review?(m_kato)

    
    

    
  


  
The Valid() check has already been done within FindOrMakeFont, so just pass the result up (either valid or null).

        Attachment #8651568 -
        Attachment is obsolete: true

        Attachment #8651568 -
        Flags: review?(m_kato)

        Attachment #8651570 -
        Flags: review?(m_kato)

        Attachment #8651570 -
        Flags: review?(m_kato) → review+

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/c9a6e3c257bc
Status: NEW → RESOLVED
Closed: 9 years ago

          status-firefox43:
          affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43

    
  
Crash Signature: gfxFontGroup::FindNonItalicFaceForChar(gfxFontFamily*, unsigned int) → [@ gfxFontGroup::FindNonItalicFaceForChar(gfxFontFamily*, unsigned int)]

    
    

    
  
This showed up on the beta 41 crash charts. Could you request an uplift?

    
    

    
  
Comment on attachment 8651570 [details] [diff] [review]
patch, skip duplicate call to Valid()

Approval Request Comment
[Feature/regressing bug #]: crasher caused by null dereference
[User impact if declined]: small number of crashes
[Describe test coverage new/current, TreeHerder]: landed on central monday
[Risks and why]: very minor fix
[String/UUID change made/needed]: none

        Attachment #8651570 -
        Flags: approval-mozilla-beta?

        Attachment #8651570 -
        Flags: approval-mozilla-aurora?

    
    

    
  
Comment on attachment 8651570 [details] [diff] [review]
patch, skip duplicate call to Valid()

Crash fix that is simple and safe. Aurora42+, Beta41+.

        Attachment #8651570 -
        Flags: approval-mozilla-beta?

        Attachment #8651570 -
        Flags: approval-mozilla-beta+

        Attachment #8651570 -
        Flags: approval-mozilla-aurora?

        Attachment #8651570 -
        Flags: approval-mozilla-aurora+

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: