From patchwork Tue Nov 5 09:41:19 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 288452 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id AAC7E2C0040 for ; Tue, 5 Nov 2013 20:41:44 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type; q=dns; s=default; b=PDlPXm4iu/oCk0ld O8GZbcceOTXAE6l9ewOt24QlYDeGNm2qHZba05MyTtKZOR+k+8IzkHxewxlMoE6M nXAdFY7QKEU8zMbOAC5nuq/AkVBnUSuNGW8ZhWpQbN+zOPm0JhgSuKslISNKesTm N/rgBQtHy+zHBHArvMOQlGVfVdM= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type; s=default; bh=Z53OyECghp0nGgS54wJjhg SlNwI=; b=EIwhXRn3Lw6/n25hdtHuj4ZWsyft3bJlJE+428HXAlnhtzIvKM79RI EhXu/1sZzqXWvWc5JGyig+7iQh+v3hIcTvcpkW7kDrjQm/RoLVZx1ZjXmHHqIvBL cqxsQLvLCWsV0cO+rcLpq0euewshWdn6/QVmrnk3ba4VnZIySgwq4= Received: (qmail 25018 invoked by alias); 5 Nov 2013 09:41:32 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 25008 invoked by uid 89); 5 Nov 2013 09:41:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_50, RDNS_NONE, SPF_HELO_PASS, SPF_PASS, URIBL_BLOCKED autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from Unknown (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 05 Nov 2013 09:41:30 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rA59fNvG023289 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 5 Nov 2013 04:41:23 -0500 Received: from localhost (ovpn-116-76.ams2.redhat.com [10.36.116.76]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id rA59fLVR030734 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 5 Nov 2013 04:41:22 -0500 Received: by localhost (Postfix, from userid 1000) id D84BB16503B; Tue, 5 Nov 2013 10:41:19 +0100 (CET) From: Dodji Seketeli To: Bernd Edlinger Cc: Jakub Jelinek , Manuel =?utf-8?B?TMOzcGV6LUliw6E=?= =?utf-8?B?w7Fleg==?= , "gcc-patches" Subject: Re: [PATCH] preprocessor/58580 - preprocessor goes OOM with warning for zero literals References: <20131031144309.GR27813@tucnak.zalov.cz> <87y559xz7y.fsf@redhat.com> <20131031173649.GW27813@tucnak.zalov.cz> <87r4awtmnx.fsf@redhat.com> <20131104115748.GO27813@tucnak.zalov.cz> <87zjpkrx8p.fsf@redhat.com> X-URL: http://www.redhat.com Date: Tue, 05 Nov 2013 10:41:19 +0100 In-Reply-To: (Bernd Edlinger's message of "Tue, 5 Nov 2013 00:43:46 +0100") Message-ID: <87k3gnqj7k.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) MIME-Version: 1.0 Bernd Edlinger writes: [...] >> if (!string_len) >> { >> string_len = 200; >> - string = XNEWVEC (char, string_len); >> + string = XCNEWVEC (char, string_len); >> } >> + else >> + memset (string, 0, string_len); > > Is this memset still necessary? Of course not ... [...] > If "ptr" is passed to get_line it will try to reallocate it, > which must fail, right? > > Maybe, this line of code is unreachable? > > Who is responsible for reallocating "string" get_line or read_line? Correct, these are real concerns. I am wondering what I was thinking. Actually, I think read_line should almost just call get_line now. Like what is done in the new version of the patch below; basically if there is a line to return, read_line just gets it (the static buffer containing the line) from get_line and returns it, otherwise the static buffer containing the last read line is left untouched and read_line returns a NULL constant. I guess this resolves the valid concern that you raised below: > If the previous invocation of read_line already had read some > characters of the following line, how is that information > recovered? How is it detected if another file is to be read this > time? Thank you very much for this thorough review. Here is the updated patch that I am bootstrapping: gcc/ChangeLog: * input.h (location_get_source_line): Take an additional line_size parameter. * input.c (get_line): New static function definition. (read_line): Take an additional line_length output parameter to be set to the size of the line. Use the new get_line function do the actual line reading. (location_get_source_line): Take an additional output line_len parameter. Update the use of read_line to pass it the line_len parameter. * diagnostic.c (adjust_line): Take an additional input parameter for the length of the line, rather than calculating it with strlen. (diagnostic_show_locus): Adjust the use of location_get_source_line and adjust_line with respect to their new signature. While displaying a line now, do not stop at the first null byte. Rather, display the zero byte as a space and keep going until we reach the size of the line. gcc/testsuite/ChangeLog: * c-c++-common/cpp/warning-zero-in-literals-1.c: New test file. --- gcc/diagnostic.c | 17 ++-- gcc/input.c | 111 ++++++++++++++++----- gcc/input.h | 3 +- .../c-c++-common/cpp/warning-zero-in-literals-1.c | Bin 0 -> 240 bytes 4 files changed, 97 insertions(+), 34 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/cpp/warning-zero-in-literals-1.c diff --git a/gcc/testsuite/c-c++-common/cpp/warning-zero-in-literals-1.c b/gcc/testsuite/c-c++-common/cpp/warning-zero-in-literals-1.c new file mode 100644 index 0000000000000000000000000000000000000000..ff2ed962ac96e47ae05b0b040f4e10b8e09637e2 GIT binary patch literal 240 zcmdPbSEyDlast_location = diagnostic->location; s = expand_location_to_spelling_point (diagnostic->location); - line = location_get_source_line (s); + line = location_get_source_line (s, &line_width); if (line == NULL) return; max_width = context->caret_max_width; - line = adjust_line (line, max_width, &(s.column)); + line = adjust_line (line, line_width, max_width, &(s.column)); pp_newline (context->printer); saved_prefix = pp_get_prefix (context->printer); pp_set_prefix (context->printer, NULL); pp_space (context->printer); - while (max_width > 0 && *line != '\0') + while (max_width > 0 && line_width > 0) { char c = *line == '\t' ? ' ' : *line; + if (c == '\0') + c = ' '; pp_character (context->printer, c); max_width--; + line_width--; line++; } pp_newline (context->printer); diff --git a/gcc/input.c b/gcc/input.c index a141a92..9526d88 100644 --- a/gcc/input.c +++ b/gcc/input.c @@ -87,53 +87,110 @@ expand_location_1 (source_location loc, return xloc; } -/* Reads one line from file into a static buffer. */ -static const char * -read_line (FILE *file) +/* This function reads a line that might contain bytes whose value is + zero. It returns the number of bytes read. The 'end-of-line' + character found at the end of the line is not contained in the + returned buffer. Note that this function has been adapted from + getline() and _IO_getdelim() GNU C library functions. It's been + duplicated here because the getline() function is not necessarily + present on all platforms. + + LINEPTR points to a buffer that is to contain the line read. + + N points to the size of the the LINEPTR buffer. + + FP points to the file to consider. */ + +static ssize_t +get_line (char **lineptr, size_t *n, FILE *fp) { - static char *string; - static size_t string_len; - size_t pos = 0; - char *ptr; + ssize_t cur_len = 0, len; + char buf[16384]; + + if (lineptr == NULL || n == NULL) + return -1; - if (!string_len) + if (*lineptr == NULL || *n == 0) { - string_len = 200; - string = XNEWVEC (char, string_len); + *n = 120; + *lineptr = XNEWVEC (char, *n); } - while ((ptr = fgets (string + pos, string_len - pos, file))) - { - size_t len = strlen (string + pos); + len = fread (buf, 1, sizeof buf, fp); + if (ferror (fp)) + return -1; - if (string[pos + len - 1] == '\n') + for (;;) + { + size_t needed; + char *t = (char*) memchr (buf, '\n', len); + if (t != NULL) len = (t - buf); + if (__builtin_expect (len >= SSIZE_MAX - cur_len, 0)) + return -1; + needed = cur_len + len + 1; + if (needed > *n) { - string[pos + len - 1] = 0; - return string; + char *new_lineptr; + if (needed < 2 * *n) + needed = 2 * *n; + new_lineptr = XRESIZEVEC (char, *lineptr, needed); + *lineptr = new_lineptr; + *n = needed; } - pos += len; - string = XRESIZEVEC (char, string, string_len * 2); - string_len *= 2; + memcpy (*lineptr + cur_len, buf, len); + cur_len += len; + if (t != NULL) + break; + len = fread (buf, 1, sizeof buf, fp); + if (ferror (fp)) + return -1; + if (len == 0) + break; } - - return pos ? string : NULL; + + if (cur_len) + (*lineptr)[cur_len] = '\0'; + return cur_len; +} + +/* Reads one line from FILE into a static buffer. LINE_LENGTH is set + by this function to the length of the returned line. Note that the + returned line can contain several zero bytes. Also note that the + returned string is allocated in static storage that is going to be + re-used by subsequent invocations of read_line. */ +static const char * +read_line (FILE *file, int *line_length) +{ + static char *string; + static size_t string_len; + + *line_length = get_line (&string, &string_len, file); + return *line_length ? string : NULL; } /* Return the physical source line that corresponds to xloc in a buffer that is statically allocated. The newline is replaced by - the null character. */ + the null character. Note that the line can contain several null + characters, so LINE_LEN, if non-null, points to the actual length + of the line. */ const char * -location_get_source_line (expanded_location xloc) +location_get_source_line (expanded_location xloc, + int *line_len) { - const char *buffer; - int lines = 1; + const char *buffer = NULL, *ptr; + int lines = 0, len = 0; FILE *stream = xloc.file ? fopen (xloc.file, "r") : NULL; if (!stream) return NULL; - while ((buffer = read_line (stream)) && lines < xloc.line) - lines++; + while ((ptr = read_line (stream, &len)) && lines < xloc.line) + { + buffer = ptr; + lines++; + if (line_len) + *line_len = len; + } fclose (stream); return buffer; diff --git a/gcc/input.h b/gcc/input.h index 8fdc7b2..128e28c 100644 --- a/gcc/input.h +++ b/gcc/input.h @@ -37,7 +37,8 @@ extern char builtins_location_check[(BUILTINS_LOCATION < RESERVED_LOCATION_COUNT) ? 1 : -1]; extern expanded_location expand_location (source_location); -extern const char *location_get_source_line (expanded_location xloc); +extern const char *location_get_source_line (expanded_location xloc, + int *line_size); extern expanded_location expand_location_to_spelling_point (source_location); extern source_location expansion_point_location_if_in_system_header (source_location);