From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 89694 invoked by alias); 13 Nov 2018 17:02:39 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 89685 invoked by uid 89); 13 Nov 2018 17:02:38 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL,BAYES_00,DATE_IN_PAST_12_24,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=no version=3.3.2 spammy=Hx-languages-length:1249, HContent-Transfer-Encoding:8bit X-HELO: mailsec113.isp.belgacom.be Received: from mailsec113.isp.belgacom.be (HELO mailsec113.isp.belgacom.be) (195.238.20.109) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 13 Nov 2018 17:02:35 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=skynet.be; i=@skynet.be; q=dns/txt; s=securemail; t=1542128555; x=1573664555; h=message-id:subject:from:to:date:in-reply-to:references: mime-version:content-transfer-encoding; bh=nBiJ5BIdnfnlZ+zi6PVQO0tESLslPAyyGk2QgNl32PM=; b=iiQGTOD6Jqs5RMyyebYozUK/sw1h6NrxvusYU98L5CeEsfvaMTiUbE2x wQ01Uu099hyX3PME/ylCxVKRgsjZNg==; Received: from 110.212-243-81.adsl-dyn.isp.belgacom.be (HELO md) ([81.243.212.110]) by relay.skynet.be with ESMTP/TLS/AES256-GCM-SHA384; 12 Nov 2018 21:47:50 +0100 Message-ID: <1542055670.1531.5.camel@skynet.be> Subject: Re: [RFAv2] Fix leak in displaced step. From: Philippe Waroquiers To: Simon Marchi , "gdb-patches@sourceware.org" Date: Tue, 13 Nov 2018 17:02:00 -0000 In-Reply-To: <516d7e99-79da-9939-7ff0-ca79942985c8@ericsson.com> References: <20181111121137.6832-1-philippe.waroquiers@skynet.be> <516d7e99-79da-9939-7ff0-ca79942985c8@ericsson.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg00220.txt.bz2 On Mon, 2018-11-12 at 16:17 +0000, Simon Marchi wrote: > From what I understand, the allocation model you propose in this patch is to > allocate a buffer the first time we do a displaced step for an inferior and > free it when the inferior exits. Yes, that is the plan. > The allocated size is > > len = gdbarch_max_insn_length (gdbarch); > > Given that there can be multiple architectures inside a single inferior, can > the required buffer size change between multiple displaced step? Good remark : I did not know of this multiple architecture for a single inferior, and then yes possibly the buffer might have to change size. So, we should rather do (unconditionally) : displaced->step_saved_copy = (gdb_byte *) xrealloc (len); > > Also, if freeing the buffer on inferior exit is indeed what we want to do, why do > we need the above cleanup? Even if the setup fails, shouldn't be fine to keep the buffer > allocated? The idea was to avoid having a piece of memory containing not properly  initialized data. But probably this is not a problem, as this data will only be used if the displaced setup is finished, at the end of the function. So, effectively no real need for the cleanup anymore. Philippe