public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Dodji Seketeli <dodji@redhat.com>
To: Bernd Edlinger <bernd.edlinger@hotmail.de>
Cc: "Jakub Jelinek" <jakub@redhat.com>,
	"Manuel López-Ibáñez" <lopezibanez@gmail.com>,
	gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] preprocessor/58580 - preprocessor goes OOM with warning for zero literals
Date: Tue, 05 Nov 2013 11:43:00 -0000	[thread overview]
Message-ID: <87eh6voz5o.fsf@redhat.com> (raw)
In-Reply-To: <DUB122-W4EB74591B27AB41AE5133E4F10@phx.gbl> (Bernd Edlinger's	message of "Tue, 5 Nov 2013 12:07:07 +0100")

Bernd Edlinger <bernd.edlinger@hotmail.de> writes:

> If you want to have at least a chance to survive something like:
>
>
> dd if=/dev/zero of=test.c bs=10240 count=10000000
>
> gcc -Wall test.c
>
>
> Then you should change the implementation of read_line to
> _not_ returning something like 100GB of zeros.

I'd say that in that case, we'd rather just die in an OOM condition and
be done with it.  Otherwise, If fear that read_line might become too
slow; you'd have to detect that the content is just zeros, for instance.

> IMHO it would be nice to limit lines returned to 10.000 bytes,
> maybe add "..." or "<line too long>" if the limit is reached.

In general, setting a limit for pathological cases like this is a good
idea, I think.  But that seems a bit ouf of the scope of this particular
bug fix; we'd need to e.g, define a new command line argument to extend
that limit if need be, for instance.  If people really feel strongly
about this I can propose a later patch to set a limit in get_line and
define a command like argument that would override that parameter.

> And maybe it would make the life of read_line's callers lots easier
> if the zero-chars are silently replaced with spaces in the returned
> line buffer.

As speed seemed to be a concern (even if, in my opinion, we are dealing
with diagnostics that are being emitted when the compilation has been
halted anyway, so we shouldn't be too concerned, unless we are talking
about pathological cases), I think that read_line should be fast by
default.  If a particular caller doesn't want to see the zeros (and thus
is ready to pay the speed price) then it can replace the zeros with
white space.  Otherwise, let's have read_line be as fast as possible.

Also keep in mind that in subsequent patches, read_line might be re-used
by e.g, gcov in nominal contexts where we don't have zeros in the middle
of the line.  In that case, speed can be a concern.

Thanks for the helpful thoughts.

-- 
		Dodji

  reply	other threads:[~2013-11-05 11:39 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-31 14:48 Bernd Edlinger
2013-10-31 15:06 ` Jakub Jelinek
2013-10-31 15:19   ` Dodji Seketeli
2013-10-31 18:26     ` Jakub Jelinek
2013-11-04 11:52       ` Dodji Seketeli
2013-11-04 11:59         ` Jakub Jelinek
2013-11-04 15:42           ` Dodji Seketeli
2013-11-05  0:10             ` Bernd Edlinger
2013-11-05  9:50               ` Dodji Seketeli
2013-11-05 11:19                 ` Bernd Edlinger
2013-11-05 11:43                   ` Dodji Seketeli [this message]
2013-11-06 22:27                 ` Bernd Edlinger
2013-11-04 12:06         ` Bernd Edlinger
2013-11-04 12:15           ` Jakub Jelinek
2013-11-04 12:32             ` Bernd Edlinger
2013-11-04 15:21           ` Dodji Seketeli
2013-11-11 10:49       ` Dodji Seketeli
2013-11-11 14:35         ` Jakub Jelinek
2013-11-11 17:13           ` Dodji Seketeli
2013-11-12 16:42             ` Dodji Seketeli
2013-11-13  5:10               ` Bernd Edlinger
2013-11-13  9:40                 ` Dodji Seketeli
2013-11-13  9:43                   ` Bernd Edlinger
2013-11-13  9:49                     ` Dodji Seketeli
2013-11-13  9:49                     ` Dodji Seketeli
2013-11-13  9:51               ` Jakub Jelinek
2013-11-14 15:12                 ` Dodji Seketeli
2013-12-09 20:11                   ` Tom Tromey
2014-01-21 12:28                   ` Bernd Edlinger
2014-01-22  8:16                     ` Dodji Seketeli
2014-01-23 17:12                       ` Jakub Jelinek
2014-01-24  2:58                         ` Bernd Edlinger
2014-01-24  7:53                         ` Dodji Seketeli
2014-01-24 15:05                       ` Markus Trippelsdorf
2014-01-24 15:41                         ` Dodji Seketeli
2014-01-24 15:44                           ` Jakub Jelinek
2014-01-24 16:09                             ` Dodji Seketeli
2014-01-24 16:13                               ` Jakub Jelinek
2014-01-24 23:02                               ` Markus Trippelsdorf
2014-01-24 23:20                                 ` Markus Trippelsdorf
2014-01-28 13:20                                   ` Dodji Seketeli
2014-01-28 13:23                                     ` Dodji Seketeli
2014-01-28 18:40                                       ` H.J. Lu
2014-01-29 11:28                                         ` Dodji Seketeli
  -- strict thread matches above, loose matches on Subject: below --
2013-10-31 13:45 Dodji Seketeli
2013-10-31 17:30 ` Manuel López-Ibáñez

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=87eh6voz5o.fsf@redhat.com \
    --to=dodji@redhat.com \
    --cc=bernd.edlinger@hotmail.de \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=lopezibanez@gmail.com \
    /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).