From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x529.google.com (mail-ed1-x529.google.com [IPv6:2a00:1450:4864:20::529]) by sourceware.org (Postfix) with ESMTPS id 909393858C53 for ; Thu, 2 Mar 2023 14:02:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 909393858C53 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ed1-x529.google.com with SMTP id s26so67836335edw.11 for ; Thu, 02 Mar 2023 06:02:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1677765765; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=oKMJDiNPYhTpDW+Gzspg8OxoAfT/DjrvQyVThvenT3Y=; b=UP4Au5pISyld5M7ruKrsEkDfk7bywGwlXbwCEfXW5kDhjf6reJ4y6BhwWeA4PJEd1I krpkkOAqByfjNm1M7ZPUB3X40F+lUQd2MCS6SOxYNFqk7kqFHa88lBd5ALijm3yX84Bm pqoLaWHLDhhbaT4WFIjHMCKmGTTylik4D8Wc6hWOa5O7OeFVAwD13oes5mHKDeF9aOnh wuWFIgRzH2rmwl0FUCzHbi7v2XhlzmtAuXocTJD7acqAv4d6Teh39lVzMZYeoJFAx3QY uRoBcxCtYV1qT2c7lmUDBm8Oo+9JMw/ow4bhAY6UCoh8F1q0wKz9ZCT1Q05GpDLrkPAR 9iTQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677765765; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=oKMJDiNPYhTpDW+Gzspg8OxoAfT/DjrvQyVThvenT3Y=; b=eU5uLIBGANY2qHHL8KMYp8CXIyyF5kDDDfR2JWHLi6+dN+/hCf1vRr+BtP+M5ejdgs FMmETUmVeNz0onn955CuaIyf/dUs/mes/ZQIa/248ugD/eXSyaQqkZEyRmQ9y8gqaBmx iP7SUA5R1WCKeZ6HC75x99JRZE2yGH444dDImCvgHC6MKO2R5N+mchLJ1mQ3IcyZ7wF/ 2kSvn6FoaxeYqkNmojqp4d9Vl10W1YUHxBLPjMAHANIH/tEAA+Z0rhO3nDaTNDC5TKDr PlpQaL4DtLYlOgCI8MizZtVa8U31XaKF+DH5w2Xeo4CmH+HUHLPZ6q+N1jCjWn0Wd5td Ww6Q== X-Gm-Message-State: AO0yUKU5XV2bw4mYMBDxI8VLJDhacG7CUAv0KcsQCXRxUkSKxMSrfyCQ i5bXtBxOgRglZfRBYjr/FMnAohOpzD8NcYnLJHQ= X-Google-Smtp-Source: AK7set9zfCUtxe/71vK9+IA5g1qrKsILwyhADhfz4v8qaTGFfD0t/H5AVqgtE7GXBidn5jr2lAvv7Ji4yWElZ2t9D90= X-Received: by 2002:a17:906:d7a6:b0:87b:dce7:c245 with SMTP id pk6-20020a170906d7a600b0087bdce7c245mr4936376ejb.3.1677765764867; Thu, 02 Mar 2023 06:02:44 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Costas Argyris Date: Thu, 2 Mar 2023 14:02:33 +0000 Message-ID: Subject: Re: [PATCH] libiberty: fix memory leak in pex-win32.c and refactor To: Richard Biener Cc: gcc-patches@gcc.gnu.org Content-Type: multipart/alternative; boundary="0000000000006e699905f5eb4806" X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: --0000000000006e699905f5eb4806 Content-Type: text/plain; charset="UTF-8" Thanks for the review. What is the next step please? Thanks, Costas On Thu, 2 Mar 2023 at 10:08, Richard Biener wrote: > On Thu, Mar 2, 2023 at 10:21 AM Costas Argyris > wrote: > > > > I forgot to mention that: > > > > 1) The CreateProcess documentation > > > > > https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa > > > > doesn't mention anything about taking ownership of this or any other > buffer passed to it. > > Thanks - thus the patch is OK. > > Thanks, > Richard. > > > 2) The cmdline buffer gets created by the argv_to_cmdline function > > > > https://github.com/gcc-mirror/gcc/blob/master/libiberty/pex-win32.c#L339 > > > > which has this comment right above it: > > > > /* Return a Windows command-line from ARGV. It is the caller's > > responsibility to free the string returned. */ > > > > Thanks, > > Costas > > > > On Thu, 2 Mar 2023 at 07:32, Richard Biener > wrote: > >> > >> On Wed, Mar 1, 2023 at 7:14 PM Costas Argyris via Gcc-patches > >> wrote: > >> > > >> > Hi > >> > > >> > It seems that the win32_spawn function in libiberty/pex-win32.c is > leaking > >> > the cmdline buffer in 2/3 exit scenarios (it is only free'd in 1/3). > The > >> > problem here is that the cleanup code is written 3 times, one at each > exit > >> > scenario. > >> > > >> > The proposed attached refactoring has the cleanup code appearing just > once > >> > and is executed for all exit scenarios, reducing the likelihood of > such > >> > leaks in the future. > >> > >> One could imagine that CreateProcess in case of success takes ownership > of > >> the buffer pointed to by cmdline? If you can confirm it is not then > the patch > >> looks OK to me. > >> > >> Thanks, > >> Richard. > >> > >> > Thanks, > >> > Costas > --0000000000006e699905f5eb4806--