[pph] Macro Validation

[Lawrence Crowl, 2011-04-07 17:54:12]

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.

The simple macro validation scheme for PTH is not sufficient for
PPH.  In particular, in libcpp, even identifiers that are skipped
in preprocessing are entered into the symbol table.  We need to
ignore these.  So, add two macro attributes, used_by_directive and
expanded_to_text.  If neither of these attributes is set, then the
macro is not used and can be ignored for validation and replay.

These changes bring the macro validation to a workable state.  There
may be some fine tuning later.

Make an inability to open a PPH file for reading a plain error rather
than a fatal error.  Fatal errors do not seem to play well with tests.

Adjust tests to reflect changes.  There is still one unexpected
failure, p1mean.cc does not compare equal in assembly.  Not yet
investigated.


gcc/testsuite/ChangeLog.pph

2011-04-06  Lawrence Crowl  <crowl@google.com>

	* g++.dg/pph/p1mean.cc: Now pass validation.
	* g++.dg/pph/p1stdlib.cc: Likewise.
	* g++.dg/pph/d1symnotinc.cc: Failure to read is now not fatal.
        * g++.dg/pph/d1chained.cc: Miscategorized as a failure, ...
        * g++.dg/pph/c1chained.cc: so it has been renamed.

gcc/cp/ChangeLog.pph

2011-04-06  Lawrence Crowl  <crowl@google.com>

        * pph.c (pth_dump_identifiers):  Split cpp_idents_used::max_length
	into max_ident_length and max_value_length.  Print used_by_directive
	and expanded_to_text attributes of macros.
	(pth_save_identifiers): Split cpp_idents_used::max_length into
	max_ident_length and max_value_length.  Filter out macro that are
	neither used_by_directive nor expanded_to_text, which requires
	precounting the number of entries remaining.  Save used_by_directive
	and expanded_to_text attributes of the macros.
	(pth_load_identifiers): Split cpp_idents_used::max_length into
	max_ident_length and max_value_length.  Restore used_by_directive and
	expanded_to_text attributes of the macros.
	(pph_read_file): Make failure to read a pph file a non-fatal error.

libcpp/ChangeLog.pph

2011-04-06  Lawrence Crowl <crowl@google.com>

        * include/cpplib.h (struct cpp_hashnode): Add used_by_directive and
	expanded_to_text attributes for macros.  Take their bits from
	directive_index, which does not need them.
        * include/symtab.h (struct cpp_idents_used): Split max_length into
	max_ident_len and max_value_len.  Reorder fields to avoid gaps.
	(struct cpp_ident_use): Add used_by_directive and expanded_to_text
	attributes for macros.
        * internal.h (struct cpp_lookaside): Split max_length into
        max_ident_len and max_value_len.
	* macro.c (enter_macro_context): Mark expanded_to_text macro attribute.
	* directives.c (DIRECTIVE_TABLE): Add sizing comment.
	(do_define): Mark used_by_directive macro attribute.
	(do_undef): Likewise.
	(do_ifdef): Likewise.
	* expr.c (parse_defined): Mark used_by_directive macro attribute.
        * 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 save used_by_directive and
	expanded_to_text attributes of macros.
        * (cpp_lt_replay): Split cpp_idents_used::max_lenth into
        max_ident_len and max_value_len.  Allocate a buffer with the sum.


Index: gcc/testsuite/g++.dg/pph/p1mean.cc
===================================================================
*** gcc/testsuite/g++.dg/pph/p1mean.cc	(revision 172001)
--- gcc/testsuite/g++.dg/pph/p1mean.cc	(working copy)
***************
*** 1,9 ****
! #include <stdlib.h> // {dg-error fails macro validation "" { xfail *-*-* } }
! #include <stdio.h> // {dg-error fails macro validation "" { xfail *-*-* } }
! #include <math.h> // {dg-error fails macro validation "" { xfail *-*-* } }
! #include <string.h> // {dg-error fails macro validation "" { xfail *-*-* } }
! // { dg-excess-errors "In file included from" { xfail *-*-* } }
! // { dg-excess-errors "assembly comparison" { xfail *-*-* } }
  
  static unsigned long long MAX_ITEMS = 10000;
  
--- 1,7 ----
! #include <stdlib.h>
! #include <stdio.h>
! #include <math.h>
! #include <string.h>
  
  static unsigned long long MAX_ITEMS = 10000;
  
Index: gcc/testsuite/g++.dg/pph/d1symnotinc.cc
===================================================================
*** gcc/testsuite/g++.dg/pph/d1symnotinc.cc	(revision 172001)
--- gcc/testsuite/g++.dg/pph/d1symnotinc.cc	(working copy)
***************
*** 1,5 ****
! // { dg-bogus "Cannot open PPH file for reading" "" { xfail *-*-* } }
  #define NAME v
  #define VALUE 1
  #include "d1symnotinc.h"
- // { dg-excess-errors "compilation terminated" { xfail *-*-* } }
--- 1,4 ----
! // { dg-error "Cannot open PPH file for reading" }
  #define NAME v
  #define VALUE 1
  #include "d1symnotinc.h"
Index: gcc/testsuite/g++.dg/pph/d1chained.cc
===================================================================
*** gcc/testsuite/g++.dg/pph/d1chained.cc	(revision 172001)
--- gcc/testsuite/g++.dg/pph/d1chained.cc	(working copy)
***************
*** 1,3 ****
- #include "c1chained1.h"
- #include "c1chained2.h" // { dg-error "fails macro validation" }
- int x = TWO;
--- 0 ----
Index: gcc/testsuite/g++.dg/pph/c1chained.cc
===================================================================
*** gcc/testsuite/g++.dg/pph/c1chained.cc	(revision 0)
--- gcc/testsuite/g++.dg/pph/c1chained.cc	(revision 0)
***************
*** 0 ****
--- 1,3 ----
+ #include "c1chained1.h"
+ #include "c1chained2.h"
+ int x = TWO;
Index: gcc/testsuite/g++.dg/pph/p1stdlib.cc
===================================================================
*** gcc/testsuite/g++.dg/pph/p1stdlib.cc	(revision 172001)
--- gcc/testsuite/g++.dg/pph/p1stdlib.cc	(working copy)
***************
*** 1,6 ****
! #include <stdlib.h> // { dg-error "fails macro validation" "" { xfail *-*-* }
}
! // { dg-excess-errors "In file included from" { xfail *-*-* } }
! // { dg-excess-errors "regular compilation failed" { xfail *-*-* } }
  
  int f(const char* s)
  {
--- 1,4 ----
! #include <stdlib.h>
  
  int f(const char* s)
  {
Index: gcc/cp/pph.c
===================================================================
*** gcc/cp/pph.c	(revision 172001)
--- gcc/cp/pph.c	(working copy)
*************** pth_save_token_cache (cp_token_cache *ca
*** 448,453 ****
--- 448,454 ----
    pph_output_uint (f, num);
    for (i = 0, tok = cache->first; i < num; tok++, i++)
      pth_save_token (tok, f);
+ 
  }
  
  
*************** static void
*** 475,483 ****
  pth_dump_identifiers (FILE *stream, cpp_idents_used *identifiers)
  {
    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;
--- 476,486 ----
  pth_dump_identifiers (FILE *stream, cpp_idents_used *identifiers)
  {
    unsigned int idx, col = 1;
+   char use, exp;
  
!   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_dump_identifiers (FILE *stream, cpp_
*** 487,503 ****
            fprintf (stream, "\n");
            col = 1;
          }
        if (ident->before_str || ident->after_str)
          {
            if (col > 1)
              fprintf (stream, "\n");
!           fprintf (stream, " %s = %s -> %s\n", ident->ident_str,
                     ident->before_str, ident->after_str);
            col = 1;
          }
        else
          {
!           fprintf (stream, " %s", ident->ident_str);
            col += ident->ident_len;
          }
      }
--- 490,508 ----
            fprintf (stream, "\n");
            col = 1;
          }
+       use = ident->used_by_directive ? 'U' : '-';
+       exp = ident->expanded_to_text ? 'E' : '-';
        if (ident->before_str || ident->after_str)
          {
            if (col > 1)
              fprintf (stream, "\n");
!           fprintf (stream, " %s %c%c = %s -> %s\n", ident->ident_str, use,
exp,
                     ident->before_str, ident->after_str);
            col = 1;
          }
        else
          {
!           fprintf (stream, " %s %c%c", ident->ident_str, use, exp);
            col += ident->ident_len;
          }
      }
*************** pth_debug_state (void)
*** 764,788 ****
  static void
  pth_save_identifiers (cpp_idents_used *identifiers, pph_stream *stream)
  {
!   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);
      }
--- 769,813 ----
  static void
  pth_save_identifiers (cpp_idents_used *identifiers, pph_stream *stream)
  {
!   unsigned int num_entries, active_entries, id;
  
    num_entries = identifiers->num_entries;
!   pph_output_uint (stream, identifiers->max_ident_len);
!   pph_output_uint (stream, identifiers->max_value_len);
  
+   active_entries = 0;
    for ( id = 0; id < num_entries; ++id )
      {
        cpp_ident_use *entry = identifiers->entries + id;
+       if (!(entry->used_by_directive || entry->expanded_to_text))
+         continue;
+       ++active_entries;
+     }
+ 
+   pph_output_uint (stream, active_entries);
+ 
+   for ( id = 0; id < num_entries; ++id )
+     {
+       cpp_ident_use *entry = identifiers->entries + id;
+ 
+       if (!(entry->used_by_directive || entry->expanded_to_text))
+         continue;
+ 
+       /* FIX pph: We are wasting space; ident_len, used_by_directive
+       and expanded_to_text together could fit into a single uint. */
  
!       pph_output_uint (stream, entry->used_by_directive);
!       pph_output_uint (stream, entry->expanded_to_text);
! 
!       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
*** 1041,1051 ****
  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);
--- 1066,1078 ----
  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);
*************** pth_load_identifiers (cpp_idents_used *i
*** 1061,1067 ****
    /* Read the identifiers in HUNK. */
    for (j = 0; j < num_entries; ++j)
      {
!       const char *s = pph_input_string (stream);
        gcc_assert (s);
        ident_len = strlen (s);
        identifiers->entries[j].ident_len = ident_len;
--- 1088,1097 ----
    /* Read the identifiers in HUNK. */
    for (j = 0; j < num_entries; ++j)
      {
!       const char *s;
!       identifiers->entries[j].used_by_directive = pph_input_uint (stream);
!       identifiers->entries[j].expanded_to_text = pph_input_uint (stream);
!       s = pph_input_string (stream);
        gcc_assert (s);
        ident_len = strlen (s);
        identifiers->entries[j].ident_len = ident_len;
*************** pph_read_file (const char *filename)
*** 1946,1957 ****
      fprintf (pph_logfile, "PPH: Reading %s\n", filename);
  
    stream = pph_stream_open (filename, "rb");
!   if (!stream)
!     fatal_error ("Cannot open PPH file for reading: %s: %m", filename);
! 
!   pph_read_file_contents (stream);
! 
!   pph_stream_close (stream);
  }
  
  /* Record a #include or #include_next for PTH.  */
--- 1976,1988 ----
      fprintf (pph_logfile, "PPH: Reading %s\n", filename);
  
    stream = pph_stream_open (filename, "rb");
!   if (stream)
!     {
!       pph_read_file_contents (stream);
!       pph_stream_close (stream);
!     }
!   else
!     error ("Cannot open PPH file for reading: %s: %m", filename);
  }
  
  /* Record a #include or #include_next for PTH.  */
Index: libcpp/symtab.c
===================================================================
*** libcpp/symtab.c	(revision 172001)
--- 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)
*** 585,601 ****
        hashnode node = table_entry->node;
        if (node)
          {
!           cpp_ident_use *summary_entry;
!           cpp_hashnode *cpp_node;
  
!           summary_entry = used.entries + summary_index++;
            summary_entry->ident_len = node->len;
            summary_entry->ident_str = (const char *)node->str;
            summary_entry->before_len = table_entry->length;
            summary_entry->before_str = table_entry->value;
  
            /* Capture any macro value.  */
-           cpp_node = CPP_HASHNODE (node);
            if (cpp_node->type == NT_MACRO)
                summary_entry->after_len = lt_macro_value
                    (&summary_entry->after_str, aside, reader, cpp_node);
--- 586,602 ----
        hashnode node = table_entry->node;
        if (node)
          {
!           cpp_ident_use *summary_entry = used.entries + summary_index++;
!           cpp_hashnode *cpp_node = CPP_HASHNODE (node);
  
!           summary_entry->used_by_directive = cpp_node->used_by_directive;
!           summary_entry->expanded_to_text = cpp_node->expanded_to_text;
            summary_entry->ident_len = node->len;
            summary_entry->ident_str = (const char *)node->str;
            summary_entry->before_len = table_entry->length;
            summary_entry->before_str = table_entry->value;
  
            /* Capture any macro value.  */
            if (cpp_node->type == NT_MACRO)
                summary_entry->after_len = lt_macro_value
                    (&summary_entry->after_str, aside, reader, cpp_node);
*************** 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/macro.c
===================================================================
*** libcpp/macro.c	(revision 172001)
--- libcpp/macro.c	(working copy)
*************** enter_macro_context (cpp_reader *pfile, 
*** 880,885 ****
--- 880,887 ----
        /* Disable the macro within its expansion.  */
        node->flags |= NODE_DISABLED;
  
+       node->expanded_to_text = 1;
+ 
        if (!(node->flags & NODE_USED))
  	{
  	  node->flags |= NODE_USED;
*************** cpp_get_token (cpp_reader *pfile)
*** 1268,1273 ****
--- 1270,1276 ----
  	break;
  
        node = result->val.node.node;
+       node->expanded_to_text = 1;
  
        if (node->type != NT_MACRO || (result->flags & NO_EXPAND))
  	break;
Index: libcpp/directives.c
===================================================================
*** libcpp/directives.c	(revision 172001)
--- libcpp/directives.c	(working copy)
*************** static void cpp_pop_definition (cpp_read
*** 137,143 ****
     pcmcia-cs-3.0.9).  This is no longer important as directive lookup
     is now O(1).  All extensions other than #warning, #include_next,
     and #import are deprecated.  The name is where the extension
!    appears to have come from.  */
  
  #define DIRECTIVE_TABLE							\
  D(define,	T_DEFINE = 0,	KANDR,     IN_I)	   /* 270554 */ \
--- 137,146 ----
     pcmcia-cs-3.0.9).  This is no longer important as directive lookup
     is now O(1).  All extensions other than #warning, #include_next,
     and #import are deprecated.  The name is where the extension
!    appears to have come from.
! 
!    Make sure the bitfield directive_index in include/cpplib.h is large
!    enough to index the entire table.  */
  
  #define DIRECTIVE_TABLE							\
  D(define,	T_DEFINE = 0,	KANDR,     IN_I)	   /* 270554 */ \
*************** do_define (cpp_reader *pfile)
*** 580,585 ****
--- 583,590 ----
        if (pfile->cb.before_define)
  	pfile->cb.before_define (pfile);
  
+       node->used_by_directive = 1;
+ 
        if (_cpp_create_definition (pfile, node))
  	if (pfile->cb.define)
  	  pfile->cb.define (pfile, pfile->directive_line, node);
*************** do_undef (cpp_reader *pfile)
*** 602,607 ****
--- 607,614 ----
        if (pfile->cb.undef)
  	pfile->cb.undef (pfile, pfile->directive_line, node);
  
+       node->used_by_directive = 1;
+ 
        /* 6.10.3.5 paragraph 2: [#undef] is ignored if the specified
  	 identifier is not currently defined as a macro name.  */
        if (node->type == NT_MACRO)
*************** do_ifdef (cpp_reader *pfile)
*** 1819,1824 ****
--- 1826,1832 ----
  
        if (node)
  	{
+           node->used_by_directive = 1;
  	  /* Do not treat conditional macros as being defined.  This is due to
  	     the powerpc and spu ports using conditional macros for 'vector',
  	     'bool', and 'pixel' to act as conditional keywords.  This messes
*************** do_ifndef (cpp_reader *pfile)
*** 1865,1870 ****
--- 1873,1879 ----
  
        if (node)
  	{
+           node->used_by_directive = 1;
  	  /* Do not treat conditional macros as being defined.  This is due to
  	     the powerpc and spu ports using conditional macros for 'vector',
  	     'bool', and 'pixel' to act as conditional keywords.  This messes
Index: libcpp/include/cpplib.h
===================================================================
*** libcpp/include/cpplib.h	(revision 172001)
--- libcpp/include/cpplib.h	(working copy)
*************** union GTY(()) _cpp_hashnode_value {
*** 651,659 ****
  struct GTY(()) cpp_hashnode {
    struct ht_identifier ident;
    unsigned int is_directive : 1;
!   unsigned int directive_index : 7;	/* If is_directive,
  					   then index into directive table.
  					   Otherwise, a NODE_OPERATOR.  */
    unsigned char rid_code;		/* Rid code - for front ends.  */
    ENUM_BITFIELD(node_type) type : 6;	/* CPP node type.  */
    unsigned int flags : 10;		/* CPP flags.  */
--- 651,661 ----
  struct GTY(()) cpp_hashnode {
    struct ht_identifier ident;
    unsigned int is_directive : 1;
!   unsigned int directive_index : 5;	/* If is_directive,
  					   then index into directive table.
  					   Otherwise, a NODE_OPERATOR.  */
+   unsigned int used_by_directive : 1;	/* In an active #if, #define etc.  */
+   unsigned int expanded_to_text : 1;	/* Produces tokens for parser.  */
    unsigned char rid_code;		/* Rid code - for front ends.  */
    ENUM_BITFIELD(node_type) type : 6;	/* CPP node type.  */
    unsigned int flags : 10;		/* CPP flags.  */
Index: libcpp/include/symtab.h
===================================================================
*** libcpp/include/symtab.h	(revision 172001)
--- libcpp/include/symtab.h	(working copy)
*************** typedef struct cpp_lookaside cpp_lookasi
*** 110,126 ****
  
  typedef struct GTY(()) cpp_ident_use
  {
-   unsigned int ident_len;
    const char *ident_str;
-   unsigned int before_len;
    const char *before_str;
-   unsigned int after_len;
    const char *after_str;
  } cpp_ident_use;
  
  typedef struct GTY(()) cpp_idents_used
  {
!   unsigned int max_length;
    unsigned int num_entries;
    cpp_ident_use *entries;
    struct obstack * GTY((skip)) strings;
--- 110,130 ----
  
  typedef struct GTY(()) cpp_ident_use
  {
    const char *ident_str;
    const char *before_str;
    const char *after_str;
+   unsigned int ident_len;
+   unsigned int before_len;
+   unsigned int after_len;
+   bool used_by_directive;
+   bool expanded_to_text;
+   /* FIX pph: We can reduce the space by shortening ident_len.  */
  } cpp_ident_use;
  
  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/expr.c
===================================================================
*** libcpp/expr.c	(revision 172001)
--- libcpp/expr.c	(working copy)
*************** parse_defined (cpp_reader *pfile)
*** 665,670 ****
--- 665,671 ----
    if (token->type == CPP_NAME)
      {
        node = token->val.node.node;
+       node->used_by_directive = 1;
        if (paren && cpp_get_token (pfile)->type != CPP_CLOSE_PAREN)
  	{
  	  cpp_error (pfile, CPP_DL_ERROR, "missing ')' after \"defined\"");
*************** eval_token (cpp_reader *pfile, const cpp
*** 803,808 ****
--- 804,810 ----
  	}
        else
  	{
+           token->val.node.node->used_by_directive = 1;
  	  result.high = 0;
  	  result.low = 0;
  	  if (CPP_OPTION (pfile, warn_undef) && !pfile->state.skip_eval)
Index: libcpp/internal.h
===================================================================
*** libcpp/internal.h	(revision 172001)
--- 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.  */

--
This patch is available for review at http://codereview.appspot.com/4379044

[Diego Novillo, 2011-04-08 13:36:49]

On Thu, Apr 7, 2011 at 13:54, Lawrence Crowl <crowl@google.com> wrote:

> *************** pth_save_token_cache (cp_token_cache *ca
> *** 448,453 ****
> --- 448,454 ----
>    pph_output_uint (f, num);
>    for (i = 0, tok = cache->first; i < num; tok++, i++)
>      pth_save_token (tok, f);
> +
>  }

Extra blank line.


>  struct GTY(()) cpp_hashnode {
>    struct ht_identifier ident;
>    unsigned int is_directive : 1;
> !   unsigned int directive_index : 5;   /* If is_directive,
>                                           then index into directive table.
>                                           Otherwise, a NODE_OPERATOR.  */
> +   unsigned int used_by_directive : 1; /* In an active #if, #define etc.  */
> +   unsigned int expanded_to_text : 1;  /* Produces tokens for parser.  */

Hm, and 5 bits are still enough for directive_index?


Diego.

[Lawrence Crowl, 2011-04-08 18:20:13]

On 4/8/11, Diego Novillo <dnovillo@google.com> wrote:
> On Thu, Apr 7, 2011 at 13:54, Lawrence Crowl <crowl@google.com> wrote:
>> *************** pth_save_token_cache (cp_token_cache *ca
>> *** 448,453 ****
>> --- 448,454 ----
>>    pph_output_uint (f, num);
>>    for (i = 0, tok = cache->first; i < num; tok++, i++)
>>      pth_save_token (tok, f);
>> +
>>  }
>
> Extra blank line.

Fixed.

>>  struct GTY(()) cpp_hashnode {
>>    struct ht_identifier ident;
>>    unsigned int is_directive : 1;
>> !   unsigned int directive_index : 5;   /* If is_directive,
>>                                           then index into directive table.
>>                                           Otherwise, a NODE_OPERATOR.  */
>> +   unsigned int used_by_directive : 1; /* In an active #if, #define etc.
>>  */
>> +   unsigned int expanded_to_text : 1;  /* Produces tokens for parser.  */
>
> Hm, and 5 bits are still enough for directive_index?

Yes there are enough.  I added a comment at the table definition
to ensure that future changes keep it so.

Pushed.

-- 
Lawrence Crowl

[tromey_redhat.com, 2011-04-08 19:36:24]

>>>>> "Lawrence" == Lawrence Crowl <crowl@google.com> writes:

Diego> Hm, and 5 bits are still enough for directive_index?

Lawrence> Yes there are enough.  I added a comment at the table definition
Lawrence> to ensure that future changes keep it so.

I suggest a static assertion of some kind.  Or in lieu of that, maybe
the field's can be made an enum somehow, and then gcc will do the
checking.

Tom