* [pph] Buffer overrun in preprocessor symbol replay
@ 2011-03-10 2:06 Lawrence Crowl
2011-03-10 15:02 ` Diego Novillo
0 siblings, 1 reply; 2+ messages in thread
From: Lawrence Crowl @ 2011-03-10 2:06 UTC (permalink / raw)
To: gcc-patches List, Diego Novillo
[-- Attachment #1: Type: text/plain, Size: 581 bytes --]
In my last PPH change, I eliminated the redundancy in the preprocessor
identifier lookaside table by removing the name of the identifier from
the head of the macro value. This later led to a buffer overrun in
libcpp/symtab.c cpp_lt_replay. The buffer was allocated based on the
value string size, which is was no longer large enough to hold the
definition string.
Split cpp_idents_used::max_length and cpp_lookaside::max_length into
max_ident_len and max_value_len. In cpp_lt_replay, allocate the
buffer based on the sum of max_ident_len and max_value_len.
--
Lawrence Crowl
[-- Attachment #2: src.change --]
[-- Type: application/octet-stream, Size: 1002 bytes --]
Index: gcc/cp/ChangeLog.pph
2011-03-09 Lawrence Crowl <crowl@google.com>
* pph.c (pth_dump_identifiers): Split cpp_idents_used::max_length
into max_ident_length and max_value_length.
(pth_save_identifiers): Likewise.
(pth_load_identifiers): Likewise.
Index: libcpp/ChangeLog.pph
2011-03-09 Lawrence Crowl <crowl@google.com>
* include/symtab.h (struct cpp_idents_used): Split max_length
into max_ident_len and max_value_len.
* internal.h (struct cpp_lookaside): Split max_length into
max_ident_len and max_value_len.
* symtab.c (cpp_lt_create): Split cpp_lookaside::max_length
into max_ident_len and max_value_len.
* (lt_macro_value): Likewise.
* (lt_lookup): Likewise.
* (cpp_lt_capture): Likewise. Also split cpp_idents_used::max_lenth
into max_ident_len and max_value_len.
* (cpp_lt_replay): Split cpp_idents_used::max_lenth into
max_ident_len and max_value_len. Allocate a buffer with the sum.
[-- Attachment #3: src.patch --]
[-- Type: text/x-patch, Size: 9783 bytes --]
Index: gcc/cp/pph.c
===================================================================
*** gcc/cp/pph.c (revision 170837)
--- gcc/cp/pph.c (working copy)
*************** pth_dump_identifiers (FILE *stream, cpp_
*** 502,509 ****
{
unsigned int idx, col = 1;
! fprintf (stream, "%u identifiers up to %u chars\n",
! identifiers->num_entries, identifiers->max_length);
for (idx = 0; idx < identifiers->num_entries; ++idx)
{
cpp_ident_use *ident = identifiers->entries + idx;
--- 502,510 ----
{
unsigned int idx, col = 1;
! fprintf (stream, "%u identifiers up to %u chars, vals to %u chars\n",
! identifiers->num_entries, identifiers->max_ident_len,
! identifiers->max_value_len);
for (idx = 0; idx < identifiers->num_entries; ++idx)
{
cpp_ident_use *ident = identifiers->entries + idx;
*************** pth_save_identifiers (cpp_idents_used *i
*** 793,814 ****
unsigned int num_entries, id;
num_entries = identifiers->num_entries;
! pph_output_uint (stream, identifiers->max_length);
pph_output_uint (stream, num_entries);
for ( id = 0; id < num_entries; ++id )
{
cpp_ident_use *entry = identifiers->entries + id;
! gcc_assert (entry->ident_len <= identifiers->max_length);
pph_output_string_with_length (stream, entry->ident_str,
entry->ident_len);
! gcc_assert (entry->before_len <= identifiers->max_length);
pph_output_string_with_length (stream, entry->before_str,
entry->before_len);
! gcc_assert (entry->after_len <= identifiers->max_length);
pph_output_string_with_length (stream, entry->after_str,
entry->after_len);
}
--- 794,816 ----
unsigned int num_entries, id;
num_entries = identifiers->num_entries;
! pph_output_uint (stream, identifiers->max_ident_len);
! pph_output_uint (stream, identifiers->max_value_len);
pph_output_uint (stream, num_entries);
for ( id = 0; id < num_entries; ++id )
{
cpp_ident_use *entry = identifiers->entries + id;
! gcc_assert (entry->ident_len <= identifiers->max_ident_len);
pph_output_string_with_length (stream, entry->ident_str,
entry->ident_len);
! gcc_assert (entry->before_len <= identifiers->max_value_len);
pph_output_string_with_length (stream, entry->before_str,
entry->before_len);
! gcc_assert (entry->after_len <= identifiers->max_value_len);
pph_output_string_with_length (stream, entry->after_str,
entry->after_len);
}
*************** static void
*** 1025,1035 ****
pth_load_identifiers (cpp_idents_used *identifiers, pph_stream *stream)
{
unsigned int j;
! unsigned int max_length, num_entries;
unsigned int ident_len, before_len, after_len;
! max_length = pph_input_uint (stream);
! identifiers->max_length = max_length;
num_entries = pph_input_uint (stream);
identifiers->num_entries = num_entries;
identifiers->entries = XCNEWVEC (cpp_ident_use, num_entries);
--- 1027,1039 ----
pth_load_identifiers (cpp_idents_used *identifiers, pph_stream *stream)
{
unsigned int j;
! unsigned int max_ident_len, max_value_len, num_entries;
unsigned int ident_len, before_len, after_len;
! max_ident_len = pph_input_uint (stream);
! identifiers->max_ident_len = max_ident_len;
! max_value_len = pph_input_uint (stream);
! identifiers->max_value_len = max_value_len;
num_entries = pph_input_uint (stream);
identifiers->num_entries = num_entries;
identifiers->entries = XCNEWVEC (cpp_ident_use, num_entries);
Index: libcpp/symtab.c
===================================================================
*** libcpp/symtab.c (revision 170837)
--- libcpp/symtab.c (working copy)
*************** cpp_lt_create (unsigned int order, unsig
*** 411,417 ****
table->sticky_order = order;
table->active = 0;
! table->max_length = 0;
table->strings = XCNEW (struct obstack);
/* Strings need no alignment. */
_obstack_begin (table->strings, 0, 0,
--- 411,418 ----
table->sticky_order = order;
table->active = 0;
! table->max_ident_len = 0;
! table->max_value_len = 0;
table->strings = XCNEW (struct obstack);
/* Strings need no alignment. */
_obstack_begin (table->strings, 0, 0,
*************** lt_macro_value (const char** string, cpp
*** 556,563 ****
const char *definition = lt_query_macro (reader, cpp_node);
size_t macro_len = strlen (definition);
*string = (const char *) obstack_copy0 (aside->strings, definition, macro_len);
! if (macro_len > aside->max_length)
! aside->max_length = macro_len;
++aside->macrovalue;
return macro_len;
}
--- 557,564 ----
const char *definition = lt_query_macro (reader, cpp_node);
size_t macro_len = strlen (definition);
*string = (const char *) obstack_copy0 (aside->strings, definition, macro_len);
! if (macro_len > aside->max_value_len)
! aside->max_value_len = macro_len;
++aside->macrovalue;
return macro_len;
}
*************** cpp_lt_capture (cpp_reader *reader)
*** 606,612 ****
/* Take the strings from the table and give to the summary. */
used.strings = aside->strings;
aside->strings = NULL;
! used.max_length = aside->max_length;
aside->iterations += slots;
++aside->empties;
--- 607,614 ----
/* Take the strings from the table and give to the summary. */
used.strings = aside->strings;
aside->strings = NULL;
! used.max_ident_len = aside->max_ident_len;
! used.max_value_len = aside->max_value_len;
aside->iterations += slots;
++aside->empties;
*************** cpp_lt_capture (cpp_reader *reader)
*** 635,641 ****
aside->active = 0;
/* Create a new string table. */
! aside->max_length = 0;
aside->strings = XCNEW (struct obstack);
/* Strings need no alignment. */
_obstack_begin (aside->strings, 0, 0,
--- 637,644 ----
aside->active = 0;
/* Create a new string table. */
! aside->max_ident_len = 0;
! aside->max_value_len = 0;
aside->strings = XCNEW (struct obstack);
/* Strings need no alignment. */
_obstack_begin (aside->strings, 0, 0,
*************** cpp_lt_replay (cpp_reader *reader, cpp_i
*** 815,821 ****
unsigned int i;
unsigned int num_entries = identifiers->num_entries;
cpp_ident_use *entries = identifiers->entries;
! char *buffer = XCNEWVEC (char, identifiers->max_length + 1);
/* Prevent the lexer from invalidating the tokens we've read so far. */
reader->keep_tokens++;
--- 818,825 ----
unsigned int i;
unsigned int num_entries = identifiers->num_entries;
cpp_ident_use *entries = identifiers->entries;
! char *buffer = XCNEWVEC (char, identifiers->max_ident_len
! + identifiers->max_value_len + 1);
/* Prevent the lexer from invalidating the tokens we've read so far. */
reader->keep_tokens++;
*************** lt_lookup (cpp_reader *reader,
*** 968,980 ****
++aside->active;
entries[index].node = node;
entries[index].hash = hash;
! if (length > aside->max_length)
! aside->max_length = length;
/* Capture any macro value. */
if (cpp_node->type == NT_MACRO)
entries[index].length = lt_macro_value
! (&entries[index].value, aside, reader, cpp_node);
/* else .value and .length are still zero from initialization. */
/* Check table load factor. */
--- 972,984 ----
++aside->active;
entries[index].node = node;
entries[index].hash = hash;
! if (length > aside->max_ident_len)
! aside->max_ident_len = length;
/* Capture any macro value. */
if (cpp_node->type == NT_MACRO)
entries[index].length = lt_macro_value
! (&entries[index].value, aside, reader, cpp_node);
/* else .value and .length are still zero from initialization. */
/* Check table load factor. */
Index: libcpp/include/symtab.h
===================================================================
*** libcpp/include/symtab.h (revision 170837)
--- libcpp/include/symtab.h (working copy)
*************** typedef struct GTY(()) cpp_ident_use
*** 120,126 ****
typedef struct GTY(()) cpp_idents_used
{
! unsigned int max_length;
unsigned int num_entries;
cpp_ident_use *entries;
struct obstack * GTY((skip)) strings;
--- 120,127 ----
typedef struct GTY(()) cpp_idents_used
{
! unsigned int max_ident_len;
! unsigned int max_value_len;
unsigned int num_entries;
cpp_ident_use *entries;
struct obstack * GTY((skip)) strings;
Index: libcpp/internal.h
===================================================================
*** libcpp/internal.h (revision 170837)
--- libcpp/internal.h (working copy)
*************** struct cpp_lookaside {
*** 341,347 ****
unsigned int sticky_order; /* For resizing when capturing the entries. */
unsigned int active; /* Number of active entries. */
struct obstack *strings; /* For macro value storage. */
! unsigned int max_length; /* Largest string encountered. */
/* Table usage statistics. */
unsigned long long searches; /* Number of calls to lt_lookup. */
--- 341,348 ----
unsigned int sticky_order; /* For resizing when capturing the entries. */
unsigned int active; /* Number of active entries. */
struct obstack *strings; /* For macro value storage. */
! unsigned int max_ident_len; /* Largest identifier encountered. */
! unsigned int max_value_len; /* Largest macro value encountered. */
/* Table usage statistics. */
unsigned long long searches; /* Number of calls to lt_lookup. */
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [pph] Buffer overrun in preprocessor symbol replay
2011-03-10 2:06 [pph] Buffer overrun in preprocessor symbol replay Lawrence Crowl
@ 2011-03-10 15:02 ` Diego Novillo
0 siblings, 0 replies; 2+ messages in thread
From: Diego Novillo @ 2011-03-10 15:02 UTC (permalink / raw)
To: Lawrence Crowl; +Cc: gcc-patches List
On 11-03-09 09:01 PM, Lawrence Crowl wrote:
> Index: gcc/cp/ChangeLog.pph
>
> 2011-03-09 Lawrence Crowl <crowl@google.com>
>
> * pph.c (pth_dump_identifiers): Split cpp_idents_used::max_length
> into max_ident_length and max_value_length.
> (pth_save_identifiers): Likewise.
> (pth_load_identifiers): Likewise.
>
> Index: libcpp/ChangeLog.pph
>
> 2011-03-09 Lawrence Crowl <crowl@google.com>
>
> * include/symtab.h (struct cpp_idents_used): Split max_length
> into max_ident_len and max_value_len.
> * internal.h (struct cpp_lookaside): Split max_length into
> max_ident_len and max_value_len.
> * symtab.c (cpp_lt_create): Split cpp_lookaside::max_length
> into max_ident_len and max_value_len.
> * (lt_macro_value): Likewise.
> * (lt_lookup): Likewise.
> * (cpp_lt_capture): Likewise. Also split cpp_idents_used::max_lenth
> into max_ident_len and max_value_len.
> * (cpp_lt_replay): Split cpp_idents_used::max_lenth into
> max_ident_len and max_value_len. Allocate a buffer with the sum.
OK with minor nit.
> unsigned int num_entries, id;
>
> num_entries = identifiers->num_entries;
> ! pph_output_uint (stream, identifiers->max_ident_len);
> ! pph_output_uint (stream, identifiers->max_value_len);
> pph_output_uint (stream, num_entries);
>
> for ( id = 0; id < num_entries; ++id )
Extra space around '(' and ')' (this was there already, but I just noticed.)
Thanks for the quick fix! Were these the 3-4 ICEs I had noticed in pth.exp?
Diego.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-03-10 15:02 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-10 2:06 [pph] Buffer overrun in preprocessor symbol replay Lawrence Crowl
2011-03-10 15:02 ` Diego Novillo
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).