Bug 28726
| Summary: | Valgrind mismatched malloc/delete @ CSSSelectorList::adoptSelectorVector() | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Jeremy Moskovich <playmobil> |
| Component: | Web Template Framework | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED DUPLICATE | ||
| Severity: | Normal | CC: | darin, deanm, mark, playmobil |
| Priority: | P2 | Keywords: | Regression |
| Version: | 528+ (Nightly build) | ||
| Hardware: | Mac | ||
| OS: | OS X 10.5 | ||
Jeremy Moskovich
Chromium bug: http://crbug.com/18984
https://bugs.webkit.org/show_bug.cgi?id=22834 (checked in as r41877) fixed a mismatched new/free Valgrind warning.
This has now reappeared as the opposite - a mismatched malloc/delete.
The problem is that the patch for the bug above changed
- fastFree(selectorVector[i])
+ // We want to free the memory (which was allocated with new), but we
+ // don't want the destructor to run since it will affect the copy
+ // we've just made. In theory this is undefined, but operator delete
+ // is only defined taking a void*, so in practice it should be ok.
+ delete reinterpret_cast<char*>(selectorVector[i]);
It appears things have moved around and the original allocation's use of operator new now calls through to FastAllocBase which calls malloc().
The bizarre part is that the delete hack used above seems to bypass FastAllocBase's operator delete (which calls free) and instead call the system library version instead, thus triggering a Valgrind warning.
We probably need to revert this part of the aforementioned patch or come up with a better long-term solution.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Darin Adler
(In reply to comment #0)
> The bizarre part is that the delete hack used above seems to bypass
> FastAllocBase's operator delete (which calls free) and instead call the system
> library version instead, thus triggering a Valgrind warning.
The reason that should not happen is because of the override of operator delete in <wtf/FastMalloc.h>. Maybe that's not working right for Chromium?
Jeremy Moskovich
From the testing I did it appears the overrides do work in Chromium/Mac. But the char* tyepcasting causes stdlib's delete operator to be called for some unknown reason.
Removing the cast causes the overridden delete to be called correctly but also calls the constructor which is exactly what we don't want to do.
Jeremy Moskovich
whoops, I meant "destructor"
Darin Adler
*** This bug has been marked as a duplicate of bug 25930 ***