From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3790 invoked by alias); 6 Sep 2011 20:29:23 -0000 Received: (qmail 3773 invoked by uid 22791); 6 Sep 2011 20:29:22 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from wallace.st-andrews.ac.uk (HELO wallace.st-andrews.ac.uk) (138.251.30.31) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 06 Sep 2011 20:29:07 +0000 Received: from unimail.st-andrews.ac.uk ([194.247.94.140]) by wallace.st-andrews.ac.uk (8.14.3/8.14.3/Debian-5) with ESMTP id p86KSvL5019710 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NOT); Tue, 6 Sep 2011 21:28:59 +0100 Received: from UOS-DUN-MBX2.st-andrews.ac.uk ([172.20.12.22]) by uos-dun-cas1 ([172.20.12.15]) with mapi id 14.01.0289.001; Tue, 6 Sep 2011 21:28:57 +0100 From: Christopher Jefferson To: Paul Pluzhnikov CC: Jonathan Wakely , Diego Novillo , "" , "" , "" Subject: Re: [patch][google/integration] Add lightweight checks to vector::begin et. al. (issue4973065) Date: Tue, 06 Sep 2011 20:34:00 -0000 Message-ID: <28A90ECE-0F80-4762-88BF-62D8797F5760@st-andrews.ac.uk> References: <20110906162854.5C60B190B10@elbrus2.mtv.corp.google.com> In-Reply-To: Content-Type: text/plain; charset="iso-8859-1" Content-ID: <38378E939CA7C74999D71C0BCB6E8F4F@st-andrews.ac.uk> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-StAndrews-MailScanner-ID: p86KSvL5019710 X-StAndrews-MailScanner: No virus detected X-StAndrews-MailScanner-SpamCheck: not spam, SpamAssassin (not cached, score=-2.499, required 5, BAYES_00 -2.60, RDNS_NONE 0.10) X-StAndrews-MailScanner-From: caj21@st-andrews.ac.uk Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-09/txt/msg00440.txt.bz2 On 6 Sep 2011, at 21:19, Paul Pluzhnikov wrote: > On Tue, Sep 6, 2011 at 12:44 PM, Jonathan Wakely = wrote: >> On 6 September 2011 20:23, Jonathan Wakely wrote: >>>=20 >>> What's a dangling vector anyway? One that has been moved from? >>=20 >> Apparently not, since a moved-from vector would pass __valid() (as >> indeed it should) >>=20 >> So I'm quite curious what bugs this catches. >=20 > The garden variety "use after free": >=20 > vector<> *v =3D new vector<>; > ... > delete v; > ... >=20 > for (it =3D v->begin(); it !=3D v->end(); ++it) // Oops! >=20 >> The existing debug mode >> catches some fairly subtle cases of user error such as comparing >> iterators from different containers, or dereferencing past-the-end >> iterators. This patch seems to catch user errors related to ... what? >> Heap corruption? Using a vector after its destructor runs? Those >> are pretty serious, un-subtle errors and not specific to vector, so >> why add code to vector (code which isn't 100% reliable if it only >> catches corruption after the memory is reused and new data written to >> it) >=20 > It is 100% percent reliable for us, due to use of a debugging allocator > which scribbles on all free()d memory. >=20 > But yes, that's one more reason why this patch is not really good for > upstream. >=20 >> to detect more general problems that can happen to any type? >=20 > We can't (easily) catch the general problem. This patch allows us to easi= ly > catch this particular instance of it. >=20 >> The fact the patch did catch real bugs doesn't mean it's a good idea, >> as you say, those bugs probably could have been caught in other ways. >=20 > Sure -- we have other ways to catch the these bugs. They are not very > practical at the moment due to their runtime overhead. >=20 > As for your other suggestion: enabling _GLIBCXX_DEBUG just for vector, > that didn't occur to me and is something I'd like to explore. It might be interesting to profile your app under _GLIBCXX_DEBUG, and see w= here the high costs are. I previously found that stable_sort had a stupidly high cost, and it turned= out with some tweaking we could get just as much protection with a lower c= ost.=20 Chris