public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "costas.argyris at gmail dot com" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug driver/107007] libiberty's win32_spawn error handling is poor
Date: Mon, 24 Oct 2022 15:22:18 +0000	[thread overview]
Message-ID: <bug-107007-4-PzQUNxdFg3@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-107007-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107007

Costas Argyris <costas.argyris at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |costas.argyris at gmail dot com

--- Comment #1 from Costas Argyris <costas.argyris at gmail dot com> ---
A potential improvement in the win32_spawn function could be to check the
length of cmdline

https://github.com/gcc-mirror/gcc/blob/master/libiberty/pex-win32.c#L625

before passing it to CreateProcess

https://github.com/gcc-mirror/gcc/blob/master/libiberty/pex-win32.c#L630

because we know from

https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa#parameters

that the second argument cannot be more than 32,767 characters.

So if we can check that before even making the call to CreateProcess, why
bother doing a call that we know will fail?

Perhaps the check could be done inside the argv_to_cmdline function itself.   
This function mentions the 32k limit in its comments

https://github.com/gcc-mirror/gcc/blob/master/libiberty/pex-win32.c#L350

but doesn't actually consider it in the code.    Looks like the length has been
decided by the time we reach

https://github.com/gcc-mirror/gcc/blob/master/libiberty/pex-win32.c#L385

in the cmdline_len variable, so it should be just a simple numerical check that
this value is less than or equal to 32,767 before doing the memory allocation. 
  If greater, it could just return NULL without allocating any memory and the
(single) caller at

https://github.com/gcc-mirror/gcc/blob/master/libiberty/pex-win32.c#L625

already deals with NULL.

      reply	other threads:[~2022-10-24 15:22 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-22  8:42 [Bug driver/107007] New: " redi at gcc dot gnu.org
2022-10-24 15:22 ` costas.argyris at gmail dot com [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-107007-4-PzQUNxdFg3@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).