[patch][google/integration] Add lightweight checks to vector::begin et. al. (issue4973065)

Jonathan Wakely jwakely.gcc@gmail.com
Tue Sep 6 21:53:00 GMT 2011


On 6 September 2011 21:52, Paul Pluzhnikov wrote:
> On Tue, Sep 6, 2011 at 1:33 PM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>
>>>  for (it = v->begin(); it != v->end(); ++it)  // Oops!
>>
>> Eurgh, the occurrence of "delete" in anything except a destructor is a
>> code smell that should have led someone to find those bugs anyway!
>> Obviously the code above is a trivial example, but it's unforgivable
>> to write that
>
> I can assure you that the actual code that exhibited these bugs was much
> subtler than that, and the bugs were not immediately obvious.
>
> Sometimes they were not immediately obvious even after running Valgrind
> on the buggy code and getting allocation/deletion/access stack traces with
> source corrdinates.
>
>>> We can't (easily) catch the general problem. This patch allows us to easily
>>> catch this particular instance of it.
>>
>> Sure, but so does adding "assert(this);" at the top of every member
>
> Sorry, no. Adding "assert(this);" does not catch any new bugs (at least
> not on Linux) -- the code would have immediately crashed on zero-page
> dereference anyway.

I don't mean for vector::begin and the other functions in that patch,
I mean in general for member functions of any type. There are plenty
of functions that wouldn't crash when called through a null pointer.
But even std:vector has member functions like that, such as max_size.

Anyway, I think we've concluded the patch is not suitable for general
use, as it has limited value without a debugging allocator that makes
pages dirty after free.



More information about the Gcc-patches mailing list