From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <SRS0=9AsS=62=gmail.com=costasargyris@sourceware.org>
Received: from mail-ed1-x533.google.com (mail-ed1-x533.google.com [IPv6:2a00:1450:4864:20::533])
	by sourceware.org (Postfix) with ESMTPS id 4839B3858D33
	for <gcc-patches@gcc.gnu.org>; Thu,  2 Mar 2023 09:21:23 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4839B3858D33
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-x533.google.com with SMTP id a25so2404199edb.0
        for <gcc-patches@gcc.gnu.org>; Thu, 02 Mar 2023 01:21:23 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=gmail.com; s=20210112; t=1677748882;
        h=cc:to:subject:message-id:date:from:in-reply-to:references
         :mime-version:from:to:cc:subject:date:message-id:reply-to;
        bh=Oh4VZP3SmHYLwGNPXpQ6uUclu+JQEzYrF4BvlHPY1K8=;
        b=HVPPnlgzVzMR4B3g5wgO5M9KA62nSf5InXF4p5F+nowP1W6CFIF9VFfa5wgPN9fFJm
         zkvilfBoKKJRLq3MpLAA7DlPalBNLlAJYbPN2Pt+n/9N4a+yt/82nPqVjisCYDMkBkrB
         TJuOY5nXuE9t89BjXrX+nMxq9/RMX6lFm+ThmFX1wf/xMGNmD9fk/nS+HHlkfPF99X4/
         AsoA8KeNGUvZFrCbs+CvKUsLdm4NUCger+wfng3msL859aqnwxMi4webSS6+xjyVInJ6
         RtJh0ykSiwQ/01BZb/6ELB5YT3x4Spcu4ISRwpmqmx9RSrciP0yQGSJ9hLd48xyGsVGo
         7yZw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=1e100.net; s=20210112; t=1677748882;
        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=Oh4VZP3SmHYLwGNPXpQ6uUclu+JQEzYrF4BvlHPY1K8=;
        b=QAvoC9xGw6C6YbdttSAVOA8k9R6KkesbZnRZg0qonlvcqoHdW8KIPqcAz1wtvh2sai
         loPXkMMiWVQ3Ndiq/CGJPItg7D9eXmjgvG0pUAGlyj4GGUcETz/RKDTdDwyydD1kUjgH
         wAePlR6oFt1nNI9TebjrU+ykt0XDKakpIWV6bntaz3UshLhtIX7XLKldGYmshLbYGByP
         r0CGCyVcu6xHFtgls8tFTuqhP/HTbm/ukb+cpSIii+ylJT9gElMFgCFvBENkugkIN3w1
         TxLBP/lcUvWZ53enipEnyMtmJwClfgMhus2YhZRBawxfdbUAsbl4z3ySEzcCYfpBCYVq
         G8xw==
X-Gm-Message-State: AO0yUKX1GfTbScsWWM4uWfFj5DfAfvIvYSX8+L6OPU8FX8sNVQNSNyf7
	QgAPfAemAzkVSC7n0c8QeNU0+VzU7F9gylxpwh8=
X-Google-Smtp-Source: AK7set9tE1SKxdqXHNHmbk5l6nJFk3pdhy9VDbEqRSkc04nPJ83Q2c6Qy3DApVpjVjrPXy0qF+ZPbCIsbTLkreRi9eo=
X-Received: by 2002:a17:906:d9ca:b0:8af:b63:b4ba with SMTP id
 qk10-20020a170906d9ca00b008af0b63b4bamr4471721ejb.3.1677748881893; Thu, 02
 Mar 2023 01:21:21 -0800 (PST)
MIME-Version: 1.0
References: <CAHyHGCn4Gk3_pB8K1SsXNzCyWMVGGs4jOtO1_L=L+qBpkDtqHg@mail.gmail.com>
 <CAFiYyc2EUQbgxrsfCnxhcWH+JXMKBEUahcCe8U5Udp2KBSRCTw@mail.gmail.com>
In-Reply-To: <CAFiYyc2EUQbgxrsfCnxhcWH+JXMKBEUahcCe8U5Udp2KBSRCTw@mail.gmail.com>
From: Costas Argyris <costas.argyris@gmail.com>
Date: Thu, 2 Mar 2023 09:21:10 +0000
Message-ID: <CAHyHGC=un9tLm3WKtmQKb5adhOJGeknWqJV95uh=shnkBv5u-A@mail.gmail.com>
Subject: Re: [PATCH] libiberty: fix memory leak in pex-win32.c and refactor
To: Richard Biener <richard.guenther@gmail.com>
Cc: gcc-patches@gcc.gnu.org
Content-Type: multipart/alternative; boundary="00000000000020ab6b05f5e75a54"
X-Spam-Status: No, score=-1.3 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: <gcc-patches.gcc.gnu.org>

--00000000000020ab6b05f5e75a54
Content-Type: text/plain; charset="UTF-8"

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.

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 <richard.guenther@gmail.com>
wrote:

> On Wed, Mar 1, 2023 at 7:14 PM Costas Argyris via Gcc-patches
> <gcc-patches@gcc.gnu.org> 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
>

--00000000000020ab6b05f5e75a54--