From patchwork Mon Sep 19 12:32:33 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandre Oliva X-Patchwork-Id: 115331 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]) by ozlabs.org (Postfix) with SMTP id 2EE69B70B7 for ; Mon, 19 Sep 2011 22:33:29 +1000 (EST) Received: (qmail 10390 invoked by alias); 19 Sep 2011 12:33:23 -0000 Received: (qmail 10375 invoked by uid 22791); 19 Sep 2011 12:33:20 -0000 X-SWARE-Spam-Status: No, hits=-6.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, SPF_HELO_PASS, T_TVD_MIME_NO_HEADERS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 19 Sep 2011 12:32:57 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p8JCWixf010644 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 19 Sep 2011 08:32:44 -0400 Received: from freie.oliva.athome.lsd.ic.unicamp.br (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p8JCWgF5015848 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 19 Sep 2011 08:32:43 -0400 Received: from livre.localdomain (livre-to-gw.oliva.athome.lsd.ic.unicamp.br [172.31.160.19]) by freie.oliva.athome.lsd.ic.unicamp.br (8.14.4/8.14.4) with ESMTP id p8JCWeMg032451; Mon, 19 Sep 2011 09:32:41 -0300 Received: from livre.localdomain (aoliva@localhost.localdomain [127.0.0.1]) by livre.localdomain (8.14.3/8.14.3/Debian-5+lenny1) with ESMTP id p8JCWdKw011137; Mon, 19 Sep 2011 09:32:39 -0300 Received: (from aoliva@localhost) by livre.localdomain (8.14.3/8.14.3/Submit) id p8JCWXo8011135; Mon, 19 Sep 2011 09:32:33 -0300 From: Alexandre Oliva To: Eric Botcazou Cc: gcc-patches@gcc.gnu.org Subject: Re: [Ada] fix potential memory corruption in annotated value cache References: <201109162245.37684.ebotcazou@adacore.com> Date: Mon, 19 Sep 2011 09:32:33 -0300 In-Reply-To: <201109162245.37684.ebotcazou@adacore.com> (Eric Botcazou's message of "Fri, 16 Sep 2011 22:45:37 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 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 On Sep 16, 2011, Eric Botcazou wrote: > Yes, modulo Jakub's remark and s/NULL/NULL_TREE for zeroing in.base.from. Thanks, here's what I've just checked in. for gcc/ada/ChangeLog from Alexandre Oliva * gcc-interface/decl.c (annotate_value): Look up expression for insertion in the cache at the end. Index: gcc/ada/gcc-interface/decl.c =================================================================== --- gcc/ada/gcc-interface/decl.c.orig 2011-09-15 03:51:42.984761174 -0300 +++ gcc/ada/gcc-interface/decl.c 2011-09-18 16:38:02.483779930 -0300 @@ -7471,23 +7471,26 @@ annotate_value (tree gnu_size) { TCode tcode; Node_Ref_Or_Val ops[3], ret; - struct tree_int_map **h = NULL; + struct tree_int_map in; int i; /* See if we've already saved the value for this node. */ if (EXPR_P (gnu_size)) { - struct tree_int_map in; + struct tree_int_map *e; + if (!annotate_value_cache) annotate_value_cache = htab_create_ggc (512, tree_int_map_hash, tree_int_map_eq, 0); in.base.from = gnu_size; - h = (struct tree_int_map **) - htab_find_slot (annotate_value_cache, &in, INSERT); + e = (struct tree_int_map *) + htab_find (annotate_value_cache, &in); - if (*h) - return (Node_Ref_Or_Val) (*h)->to; + if (e) + return (Node_Ref_Or_Val) e->to; } + else + in.base.from = NULL_TREE; /* If we do not return inside this switch, TCODE will be set to the code to use for a Create_Node operand and LEN (set above) will be @@ -7588,8 +7591,17 @@ annotate_value (tree gnu_size) ret = Create_Node (tcode, ops[0], ops[1], ops[2]); /* Save the result in the cache. */ - if (h) + if (in.base.from) { + struct tree_int_map **h; + /* We can't assume the hash table data hasn't moved since the + initial look up, so we have to search again. Allocating and + inserting an entry at that point would be an alternative, but + then we'd better discard the entry if we decided not to cache + it. */ + h = (struct tree_int_map **) + htab_find_slot (annotate_value_cache, &in, INSERT); + gcc_assert (!*h); *h = ggc_alloc_tree_int_map (); (*h)->base.from = gnu_size; (*h)->to = ret;