This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Allocate extra 16 bytes for -fsanitize=address


On Fri, Nov 23, 2012 at 11:50 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Nov 23, 2012 at 11:33:37AM -0800, H.J. Lu wrote:
>> 2012-11-21  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>       PR bootstrap/55380
>>       * charset.c (_cpp_convert_input): Clear CPP_PAD_BUFFER_SIZE
>>       bytes if CLEAR_CPP_PAD_BUFFER isn't 0.  Allocate extra
>>       CPP_PAD_BUFFER_SIZE bytes and clear it if CLEAR_CPP_PAD_BUFFER
>>       isn't 0.
>>
>>       * files.c (read_file_guts): Allocate extra CPP_PAD_BUFFER_SIZE
>>       bytes.
>>
>>       * internal.h (CPP_PAD_BUFFER_SIZE): New.  Defined to 16 for
>>       -fsanitize=address if __SANITIZE_ADDRESS__ is defined.
>>       (CLEAR_CPP_PAD_BUFFER): New.
>
> I'd say you are making this way too much complicated.
> The following patch just changes those + 1 to + 16, adds memset of the 16
> bytes unconditionally, but as it also fixes a thinko which IMHO affects
> the most common cases (regular file, with no conversion needed), I'd say
> it ought to be faster than the old version (the old version would allocate
> st.st_size + 1 bytes long buffer, read st.st_size bytes into it,
> call _cpp_convert_input with len st.st_size and size st.st_size (the latter
> should have been st.st_size + 1, i.e. the allocated size) and thus
> to.len >= to.asize was true and we called realloc with the same length
> as malloc has been called originally.
>
> 2012-11-23  Jakub Jelinek  <jakub@redhat.com>
>
>         PR bootstrap/55380
>         * files.c (read_file_guts): Allocate extra 16 bytes instead of
>         1 byte at the end of buf.  Pass size + 16 instead of size
>         to _cpp_convert_input.
>         * charset.c (_cpp_convert_input): Reallocate if there aren't
>         at least 16 bytes beyond to.len in the buffer.  Clear 16 bytes
>         at to.text + to.len.
>
> --- libcpp/files.c.jj   2012-11-22 11:04:24.000000000 +0100
> +++ libcpp/files.c      2012-11-23 20:37:03.146379853 +0100
> @@ -671,7 +671,7 @@ read_file_guts (cpp_reader *pfile, _cpp_
>         the majority of C source files.  */
>      size = 8 * 1024;
>
> -  buf = XNEWVEC (uchar, size + 1);
> +  buf = XNEWVEC (uchar, size + 16);
>    total = 0;
>    while ((count = read (file->fd, buf + total, size - total)) > 0)
>      {
> @@ -682,7 +682,7 @@ read_file_guts (cpp_reader *pfile, _cpp_
>           if (regular)
>             break;
>           size *= 2;
> -         buf = XRESIZEVEC (uchar, buf, size + 1);
> +         buf = XRESIZEVEC (uchar, buf, size + 16);
>         }
>      }
>
> @@ -699,7 +699,7 @@ read_file_guts (cpp_reader *pfile, _cpp_
>
>    file->buffer = _cpp_convert_input (pfile,
>                                      CPP_OPTION (pfile, input_charset),
> -                                    buf, size, total,
> +                                    buf, size + 16, total,
>                                      &file->buffer_start,
>                                      &file->st.st_size);
>    file->buffer_valid = true;
> --- libcpp/charset.c.jj 2011-01-06 10:22:00.000000000 +0100
> +++ libcpp/charset.c    2012-11-23 20:40:39.736116642 +0100
> @@ -1,6 +1,6 @@
>  /* CPP Library - charsets
>     Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2006, 2008, 2009,
> -   2010 Free Software Foundation, Inc.
> +   2010, 2012 Free Software Foundation, Inc.
>
>     Broken out of c-lex.c Apr 2003, adding valid C99 UCN ranges.
>
> @@ -1729,9 +1729,12 @@ _cpp_convert_input (cpp_reader *pfile, c
>      iconv_close (input_cset.cd);
>
>    /* Resize buffer if we allocated substantially too much, or if we
> -     haven't enough space for the \n-terminator.  */
> -  if (to.len + 4096 < to.asize || to.len >= to.asize)
> -    to.text = XRESIZEVEC (uchar, to.text, to.len + 1);
> +     haven't enough space for the \n-terminator or following
> +     15 bytes of padding.  */
> +  if (to.len + 4096 < to.asize || to.len + 16 > to.asize)
> +    to.text = XRESIZEVEC (uchar, to.text, to.len + 16);
> +
> +  memset (to.text + to.len, '\0', 16);
>
>    /* If the file is using old-school Mac line endings (\r only),
>       terminate with another \r, not an \n, so that we do not mistake
>
>
>         Jakub

You should also mention:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54691

in ChangeLog entry.

Thanks.


-- 
H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]