From patchwork Tue Jun 16 15:31:57 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Martin_Li=C5=A1ka?= X-Patchwork-Id: 485067 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 67B251401DA for ; Wed, 17 Jun 2015 01:32:11 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=QG++XNHm; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:references :in-reply-to:content-type; q=dns; s=default; b=tSAgw5KORSpAeKZZu FviH9XWlL2MEDuxHRihfJx6kv9IFSdtv9qZH363YDM8nadV/8UKgWytxkMqZD3lA DbuNcHEG6A0Sp31ZUqBqzS0EDxpo395sPz7qQjVKBWpXg+oPkl0lPJN+N5WV61Uo hPL4zLAJ7oOJNQkK6j5tx4ZYZQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:references :in-reply-to:content-type; s=default; bh=KSg8LwnMKFVEoDnFRnT5Sxi lCUw=; b=QG++XNHm1uo33GRe/M+4c6AF96ynKtMj0URuf9xCm9DMtFQZOsWfJEz oZC1t2yhZoeDuFxB67if5UuEOEaBEO1WCBwB659ZJennL2U7YDcyBO4Yfx3w1+4I +zw7ZOyK2AwvU4D2bkY0BRX36MWOFSzOkVPYzZF3twwthzfc/KTg= Received: (qmail 40840 invoked by alias); 16 Jun 2015 15:32:04 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 40827 invoked by uid 89); 16 Jun 2015 15:32:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.8 required=5.0 tests=AWL, BAYES_50, KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 X-HELO: mx2.suse.de Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Tue, 16 Jun 2015 15:32:00 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 7A4FDAC62 for ; Tue, 16 Jun 2015 15:31:57 +0000 (UTC) Message-ID: <5580416D.8050009@suse.cz> Date: Tue, 16 Jun 2015 17:31:57 +0200 From: =?UTF-8?B?TWFydGluIExpxaFrYQ==?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: gcc-patches@gcc.gnu.org Subject: Re: New type-based pool allocator code miscompiled due to aliasing issue? References: <20150611165115.E278EEDC@oc7340732750.ibm.com> <60B0A9AA-8E0A-4538-8950-46224E08F1EA@gmail.com> <20150611175036.GJ10247@tucnak.redhat.com> <66F626EF-C978-4DB4-9474-185A2FC41047@gmail.com> <557E962C.5060501@suse.cz> <557EC3A0.2080303@suse.cz> <557FEBC9.8080002@suse.cz> <558026BB.7060208@suse.cz> In-Reply-To: X-IsSubscribed: yes On 06/16/2015 04:02 PM, Richard Biener wrote: > On Tue, Jun 16, 2015 at 3:38 PM, Martin Liška wrote: >> On 06/16/2015 03:17 PM, Richard Biener wrote: >>> On Tue, Jun 16, 2015 at 11:26 AM, Martin Liška wrote: >>>> On 06/15/2015 07:31 PM, Marc Glisse wrote: >>>>> On Mon, 15 Jun 2015, Martin Liška wrote: >>>>> >>>>>> Ah, I overlooked that it's not a placement new, but just static casting. >>>>>> Anyway, if I added: >>>>>> >>>>>> cselib_val () {} >>>>>> >>>>>> to struct cselib_val and changed the cast to placement new: >>>>>> char *ptr = (char *) header; >>>>>> return new (ptr) T (); >>>>>> >>>>>> I got following compilation error: >>>>>> >>>>>> In file included from ../../gcc/alias.c:46:0: >>>>>> ../../gcc/alloc-pool.h: In instantiation of ‘T* pool_allocator::allocate() [with T = cselib_val]’: >>>>>> ../../gcc/cselib.h:51:27: required from here >>>>>> ../../gcc/alloc-pool.h:416:23: error: no matching function for call to ‘cselib_val::operator new(sizetype, char*&)’ >>>>>> return new (ptr) T (); >>>>>> ^ >>>>>> In file included from ../../gcc/alias.c:47:0: >>>>>> ../../gcc/cselib.h:49:16: note: candidate: static void* cselib_val::operator new(size_t) >>>>>> inline void *operator new (size_t) >>>>>> ^ >>>>>> ../../gcc/cselib.h:49:16: note: candidate expects 1 argument, 2 provided >>>>> >>>>> #include >>>>> >>>> >>>> Hi. >>>> >>>> header file is not missing (explicit addition of the file does not help). >>>> Feel free to play with following patch which should fix cselib.h compilation error. >>> >>> cselib_val overrides the new operator but fails to provide an overload >>> for the placement new >>> form. Fix that and it should work (of course it gets quite awkward >>> with its 'new' calling >>> pool.allocate and its placement new doing value-construction then...) >>> >>> Richard. >> >> Do you mean Richard following changes: >> >> alloc-pool.h (allocate): >> ... >> + /* Placement new contructor. */ >> + inline void *operator new (size_t, elt_loc_list *&ptr) >> + { >> + return ptr; >> + } > > That should be there with including > >> and e.g. cselib.h: >> >> struct cselib_val >> { >> /* Pool allocation new operator. */ >> inline void *operator new (size_t) >> { >> return pool.allocate (); >> } >> >> /* Placement new contructor. */ >> inline void *operator new (size_t, char *&ptr) >> { >> return ptr; >> } > > Yes, though I wonder whether cselib_val should really have undefined > contents after > allocating it? (or does the pool allocator zero the memory?) > > Richard. Hio. I've added calling of placement new operators and memset a memory, look the patch works for me. If it's the right way, I'll write Changelog and run testsuite. Thanks, Martin > >> } >> >> Thanks, >> Martin >> >> >> >>> >>>> Thanks, >>>> Martin >> From d60bc8fa02161df64ddbb6bdb35c733af5e073c6 Mon Sep 17 00:00:00 2001 From: mliska Date: Tue, 16 Jun 2015 17:28:27 +0200 Subject: [PATCH] Add placement new for classes in pool allocator. --- gcc/alloc-pool.h | 10 +++++++++- gcc/asan.c | 6 ++++++ gcc/cselib.c | 6 ++++++ gcc/cselib.h | 15 +++++++++++++++ gcc/dse.c | 30 ++++++++++++++++++++++++++++++ gcc/et-forest.c | 6 ++++++ gcc/et-forest.h | 8 ++++++++ gcc/ira-color.c | 6 ++++++ gcc/lra-int.h | 18 ++++++++++++++++++ gcc/regcprop.c | 6 ++++++ gcc/tree-sra.c | 12 ++++++++++++ gcc/var-tracking.c | 21 +++++++++++++++++++++ 12 files changed, 143 insertions(+), 1 deletion(-) diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h index 1785df5..237ece3 100644 --- a/gcc/alloc-pool.h +++ b/gcc/alloc-pool.h @@ -413,7 +413,15 @@ pool_allocator::allocate () VALGRIND_DISCARD (VALGRIND_MAKE_MEM_UNDEFINED (header, size)); /* Call default constructor. */ - return (T *)(header); + T *ptr = (T *)header; + + if (!m_ignore_type_size) + { + memset (header, 0, sizeof (T)); + return new (ptr) T (); + } + else + return ptr; } /* Puts PTR back on POOL's free list. */ diff --git a/gcc/asan.c b/gcc/asan.c index 599c822..a6160f7 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -361,6 +361,12 @@ struct asan_mem_ref /* The size of the access. */ HOST_WIDE_INT access_size; + /* Placement new contructor. */ + inline void *operator new (size_t, asan_mem_ref *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { diff --git a/gcc/cselib.c b/gcc/cselib.c index 7ccaab4..9873aa0 100644 --- a/gcc/cselib.c +++ b/gcc/cselib.c @@ -53,6 +53,12 @@ struct elt_list struct elt_list *next; cselib_val *elt; + /* Placement new contructor. */ + inline void *operator new (size_t, elt_list *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { diff --git a/gcc/cselib.h b/gcc/cselib.h index cdd06ad..9ef95e3 100644 --- a/gcc/cselib.h +++ b/gcc/cselib.h @@ -48,6 +48,12 @@ struct cselib_val return pool.allocate (); } + /* Placement new contructor. */ + inline void *operator new (size_t, cselib_val *&ptr) + { + return ptr; + } + /* Delete operator utilizing pool allocation. */ inline void operator delete (void *ptr) { @@ -67,12 +73,21 @@ struct elt_loc_list { /* The insn that made the equivalence. */ rtx_insn *setting_insn; + /* Default constructor. */ + elt_loc_list () {} + /* Pool allocation new operator. */ inline void *operator new (size_t) { return pool.allocate (); } + /* Placement new contructor. */ + inline void *operator new (size_t, elt_loc_list *&ptr) + { + return ptr; + } + /* Delete operator utilizing pool allocation. */ inline void operator delete (void *ptr) { diff --git a/gcc/dse.c b/gcc/dse.c index d7d4ba6..9a6137a 100644 --- a/gcc/dse.c +++ b/gcc/dse.c @@ -345,6 +345,12 @@ struct read_info_type /* The next read_info for this insn. */ struct read_info_type *next; + /* Placement new contructor. */ + inline void *operator new (size_t, read_info_type *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { @@ -449,6 +455,12 @@ struct insn_info_type at active_local_stores. */ struct insn_info_type * next_local_store; + /* Placement new contructor. */ + inline void *operator new (size_t, insn_info_type *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { @@ -529,6 +541,12 @@ struct dse_bb_info_type accidentally clobber live hard regs. */ bitmap regs_live; + /* Placement new contructor. */ + inline void *operator new (size_t, dse_bb_info_type *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { @@ -615,6 +633,12 @@ struct group_info int *offset_map_n, *offset_map_p; int offset_map_size_n, offset_map_size_p; + /* Placement new contructor. */ + inline void *operator new (size_t, group_info *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { @@ -655,6 +679,12 @@ struct deferred_change struct deferred_change *next; + /* Placement new contructor. */ + inline void *operator new (size_t, deferred_change *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { diff --git a/gcc/et-forest.c b/gcc/et-forest.c index 57c9916..d33ceb2 100644 --- a/gcc/et-forest.c +++ b/gcc/et-forest.c @@ -56,6 +56,12 @@ struct et_occ struct et_occ *min_occ; /* The occurrence in the subtree with the minimal depth. */ + /* Placement new contructor. */ + inline void *operator new (size_t, et_occ *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { diff --git a/gcc/et-forest.h b/gcc/et-forest.h index 15c582d..4b2fb97 100644 --- a/gcc/et-forest.h +++ b/gcc/et-forest.h @@ -67,6 +67,14 @@ struct et_node struct et_occ *rightmost_occ; /* The rightmost occurrence. */ struct et_occ *parent_occ; /* The occurrence of the parent node. */ + inline et_node () {} + + /* Placement new contructor. */ + inline void *operator new (size_t, et_node *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { diff --git a/gcc/ira-color.c b/gcc/ira-color.c index b9e1bda..2265caf 100644 --- a/gcc/ira-color.c +++ b/gcc/ira-color.c @@ -113,6 +113,12 @@ struct update_cost_record /* Next record for given allocno. */ struct update_cost_record *next; + /* Placement new contructor. */ + inline void *operator new (size_t, update_cost_record *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { diff --git a/gcc/lra-int.h b/gcc/lra-int.h index 25bd3ce..800e0bb 100644 --- a/gcc/lra-int.h +++ b/gcc/lra-int.h @@ -55,6 +55,12 @@ struct lra_live_range /* Pointer to structures with the same start. */ lra_live_range_t start_next; + /* Placement new contructor. */ + inline void *operator new (size_t, lra_live_range *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { @@ -85,6 +91,12 @@ struct lra_copy /* Next copy with correspondingly REGNO1 and REGNO2. */ lra_copy_t regno1_next, regno2_next; + /* Placement new contructor. */ + inline void *operator new (size_t, lra_copy *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { @@ -208,6 +220,12 @@ struct lra_insn_reg /* Next reg info of the same insn. */ struct lra_insn_reg *next; + /* Placement new contructor. */ + inline void *operator new (size_t, lra_insn_reg *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { diff --git a/gcc/regcprop.c b/gcc/regcprop.c index 8e6452e..3126547 100644 --- a/gcc/regcprop.c +++ b/gcc/regcprop.c @@ -59,6 +59,12 @@ struct queued_debug_insn_change rtx *loc; rtx new_rtx; + /* Placement new contructor. */ + inline void *operator new (size_t, queued_debug_insn_change *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c index 9bfcd98..a14a572 100644 --- a/gcc/tree-sra.c +++ b/gcc/tree-sra.c @@ -289,6 +289,12 @@ struct access caller. */ unsigned grp_not_necessarilly_dereferenced : 1; + /* Placement new contructor. */ + inline void *operator new (size_t, access *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { @@ -319,6 +325,12 @@ struct assign_link struct access *lacc, *racc; struct assign_link *next; + /* Placement new contructor. */ + inline void *operator new (size_t, assign_link *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c index c3adf51..4227624 100644 --- a/gcc/var-tracking.c +++ b/gcc/var-tracking.c @@ -271,6 +271,12 @@ typedef struct attrs_def /* Offset from start of DECL. */ HOST_WIDE_INT offset; + /* Placement new contructor. */ + inline void *operator new (size_t, attrs_def *&ptr) + { + return ptr; + } + /* Pool allocation new operator. */ inline void *operator new (size_t) { @@ -302,6 +308,11 @@ typedef struct location_chain_def /* Initialized? */ enum var_init_status init; + /* Placement new contructor. */ + inline void *operator new (size_t, location_chain_def *&ptr) + { + return ptr; + } /* Pool allocation new operator. */ inline void *operator new (size_t) { @@ -334,6 +345,11 @@ typedef struct loc_exp_dep_s the doubly-linked list. */ struct loc_exp_dep_s **pprev; + /* Placement new contructor. */ + inline void *operator new (size_t, loc_exp_dep_s *&ptr) + { + return ptr; + } /* Pool allocation new operator. */ inline void *operator new (size_t) { @@ -588,6 +604,11 @@ typedef struct shared_hash_def /* Actual hash table. */ variable_table_type *htab; + /* Placement new contructor. */ + inline void *operator new (size_t, shared_hash_def *&ptr) + { + return ptr; + } /* Pool allocation new operator. */ inline void *operator new (size_t) { -- 2.1.4