Closed Bug 414764 Opened 12 years ago Closed 12 years ago

CToken freed too many times with <table>m</p>

Categories

(Core :: HTML: Parser, defect, P2)

x86
Windows XP
defect

Tracking

()

RESOLVED FIXED
mozilla1.9beta4

People

(Reporter: martijn.martijn, Assigned: mrbkap)

References

Details

(Keywords: testcase)

Attachments

(1 file)

I get this output with current debug trunk build and trace-malloc enabled:
== BloatView: ALL (cumulative) LEAK STATISTICS

     |<----------------Class--------------->|<-----Bytes------>|<----------------Objects---------------->|<--------------References-------------->|
                                              Per-Inst   Leaked    Total      Rem      Mean       StdDev     Total      Rem      Mean       StdDev
   0 TOTAL                                          23        0   503690        0 ( 1065.91 +/-  1396.41)  1065960       -1 ( 1365.49 +/-  2149.51)
  15 CToken                                         24        0      770        0 (  150.72 +/-   110.86)     1247       -1 (  148.69 +/-   108.09)


I'm a bit confused by this. CToken is mentioned, but it doesn't seem to have been leaking, but why is it then shown? Or is this still a memory leak?
Oops, this is a bug caused by the fix for bug 142965. It's the opposite of a leak, we're freeing a token one too many times.
Blocks: 142965
Attached patch FixSplinter Review
I can't get enough context in the diff (even with -U8) but if you look at the file, you'll see a corresponding IF_HOLD in the other case.
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #300280 - Flags: superreview?(jst)
Attachment #300280 - Flags: review?(jst)
I doubt this is exploitable, but we should probably fix it.
Flags: blocking1.9?
Priority: -- → P2
Target Milestone: --- → mozilla1.9beta4
Keywords: testcase
Summary: Memory leak with <table>m</p>? → CToken freed too many times with <table>m</p>
Attachment #300280 - Flags: superreview?(jst)
Attachment #300280 - Flags: superreview+
Attachment #300280 - Flags: review?(jst)
Attachment #300280 - Flags: review+
Comment on attachment 300280 [details] [diff] [review]
Fix

This doesn't need to go in for b3, but it's a pretty safe code correctness fix.
Attachment #300280 - Flags: approval1.9?
Sounds scary as these thing generally are exploitable. So unless we're 100% sure this one is not we should really fix it.
Flags: blocking1.9? → blocking1.9+
Comment on attachment 300280 [details] [diff] [review]
Fix

When in doubt with mem - fix it
Attachment #300280 - Flags: approval1.9? → approval1.9+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.