I'm getting the following pph test failure output after this patch
(I'll commit my patch on top of it anyways as I get the same errors
with a clean build and with my patch, which itself had a clean test
output before this recent pull before my commit).
XPASS: g++.dg/pph/x7rtti.cc -fno-dwarf2-cfi-asm -fpph-map=pph.map -I.
(test for bogus messages, line )
XPASS: g++.dg/pph/x7rtti.cc -fno-dwarf2-cfi-asm -fpph-map=pph.map -I.
(test for bogus messages, line )
XPASS: g++.dg/pph/x7rtti.cc -fno-dwarf2-cfi-asm -fpph-map=pph.map -I.
(test for bogus messages, line )
XPASS: g++.dg/pph/x7rtti.cc -fno-dwarf2-cfi-asm -fpph-map=pph.map -I.
(test for bogus messages, line )
XPASS: g++.dg/pph/x7rtti.cc -fno-dwarf2-cfi-asm -fpph-map=pph.map -I.
(test for bogus messages, line )
XPASS: g++.dg/pph/x7rtti.cc -fno-dwarf2-cfi-asm -fpph-map=pph.map -I.
(test for bogus messages, line )
XPASS: g++.dg/pph/x7rtti.cc -fno-dwarf2-cfi-asm -fpph-map=pph.map -I.
(test for bogus messages, line )
XPASS: g++.dg/pph/x7rtti.cc -fno-dwarf2-cfi-asm -fpph-map=pph.map -I.
(test for bogus messages, line )
XPASS: g++.dg/pph/x7rtti.cc -fno-dwarf2-cfi-asm -fpph-map=pph.map -I.
(test for bogus messages, line )
XPASS: g++.dg/pph/x7rtti.cc -fno-dwarf2-cfi-asm -fpph-map=pph.map -I.
(test for bogus messages, line )
XPASS: g++.dg/pph/x7rtti.cc -fno-dwarf2-cfi-asm -fpph-map=pph.map -I.
(test for bogus messages, line )
XPASS: g++.dg/pph/x7rtti.cc -fno-dwarf2-cfi-asm -fpph-map=pph.map -I.
(test for bogus messages, line )
XPASS: g++.dg/pph/x7rtti.cc -fno-dwarf2-cfi-asm -fpph-map=pph.map -I.
(test for bogus messages, line )
# of expected passes 336
# of unexpected successes 35
# of expected failures 45
# of unresolved testcases 1
Cheers,
Gab
On Thu, Aug 25, 2011 at 3:30 PM, Diego Novillo <dnovi...@google.com> wrote:
>
> To prevent PPH images from depend on the context in which they were
> generated, we require that the header file be compiled in isolation
> and when included, it should only be included in the global context.
> That is, things like
>
> namespace Foo {
> #include "bar.h"
> ...
> };
>
> should prevent bar.h from becoming a PPH image, since all the symbols
> defined in it would belong to Foo, but when bar.pph was generated,
> they belonged to ::
>
> This patch detects the use of PPH images inside nested scopes like
> that. It is a bit crude, but it works. During lexing, it keeps track
> of open and close braces (all kinds, not just { }), so when the
> #include command is found, it rejects the image if the nesting level
> is positive.
>
> Jason, I added a field to scope_chain. That seemed the cleaner
> approach to keep track of the nesting level. Is that a good place to
> keep track of it? This is the kind of bookkeeping that scope_chain
> seems to be used for, but I can put it elsewhere if you want.
>
> This error flagged the usage of sys/types.pph. This file is included
> inside 'extern "C" { }', so strictly speaking it should be rejected.
> We can relax these restrictions later.
>
>
> Tested on x86_64. Applied to branch.
>
>
> * cp-tree.h (struct saved_scope): Add field x_brace_nesting.
> * parser.c (cp_lexer_token_is_open_brace): New.
> (cp_lexer_token_is_close_brace): New.
> (cp_lexer_get_preprocessor_token): Call them.
> Increase scope_chain->x_brace_nesting for open braces, decrease
> for closing braces.
> * pph.c (pph_is_valid_here): New. Return false if
> scope_chain->x_brace_nesting is greater than 0.
> (pph_include_handler): Call it.
>
>
> testsuite/ChangeLog.pph
>
> * g++.dg/pph/pph.exp: Do not create a PPH image for sys/types.h.
> * g++.dg/pph/y8inc-nmspc.cc: Mark fixed.
>
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 8fb2ccc..e0b67c6 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -973,6 +973,10 @@ struct GTY(()) saved_scope {
> cp_binding_level *bindings;
>
> struct saved_scope *prev;
> +
> + /* Used during lexing to validate where PPH images are included, it
> + keeps track of nested bracing. */
> + unsigned x_brace_nesting;
> };
>
> /* The current open namespace. */
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 009922e..919671c 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -748,6 +748,31 @@ cp_lexer_saving_tokens (const cp_lexer* lexer)
> return VEC_length (cp_token_position, lexer->saved_tokens) != 0;
> }
>
> +
> +/* Return true if TOKEN is one of CPP_OPEN_SQUARE, CPP_OPEN_BRACE or
> + CPP_OPEN_PAREN. */
> +
> +static inline bool
> +cp_lexer_token_is_open_brace (cp_token *token)
> +{
> + return token->type == CPP_OPEN_SQUARE
> + || token->type == CPP_OPEN_BRACE
> + || token->type == CPP_OPEN_PAREN;
> +}
> +
> +
> +/* Return true if TOKEN is one of CPP_CLOSE_SQUARE, CPP_CLOSE_BRACE or
> + CPP_CLOSE_PAREN. */
> +
> +static inline bool
> +cp_lexer_token_is_close_brace (cp_token *token)
> +{
> + return token->type == CPP_CLOSE_SQUARE
> + || token->type == CPP_CLOSE_BRACE
> + || token->type == CPP_CLOSE_PAREN;
> +}
> +
> +
> /* Store the next token from the preprocessor in *TOKEN. Return true
> if we reach EOF. If LEXER is NULL, assume we are handling an
> initial #pragma pch_preprocess, and thus want the lexer to return
> @@ -835,8 +860,13 @@ cp_lexer_get_preprocessor_token (cp_lexer *lexer,
> cp_token *token)
> TREE_INT_CST_LOW (token->u.value));
> token->u.value = NULL_TREE;
> }
> + else if (cp_lexer_token_is_open_brace (token))
> + scope_chain->x_brace_nesting++;
> + else if (cp_lexer_token_is_close_brace (token))
> + scope_chain->x_brace_nesting--;
> }
>
> +
> /* Update the globals input_location and the input file stack from TOKEN. */
> static inline void
> cp_lexer_set_source_position_from_token (cp_token *token)
> diff --git a/gcc/cp/pph.c b/gcc/cp/pph.c
> index 9b1c63c..cbd9c24 100644
> --- a/gcc/cp/pph.c
> +++ b/gcc/cp/pph.c
> @@ -94,11 +94,30 @@ pph_dump_namespace (FILE *file, tree ns)
> }
>
>
> +/* Return true if PPH image NAME can be used at the point of inclusion
> + (given by LOC). */
> +
> +static bool
> +pph_is_valid_here (const char *name, location_t loc)
> +{
> + /* If we are inside a scope, reject the image. We could be inside a
> + namespace or a structure which changes the parsing context for
> + the original text file. */
> + if (scope_chain->x_brace_nesting > 0)
> + {
> + error_at (loc, "PPH file %s not included at global scope", name);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +
> /* Record a #include or #include_next for PPH. */
>
> static bool
> pph_include_handler (cpp_reader *reader,
> - location_t loc ATTRIBUTE_UNUSED,
> + location_t loc,
> const unsigned char *dname,
> const char *name,
> int angle_brackets,
> @@ -117,7 +136,9 @@ pph_include_handler (cpp_reader *reader,
>
> read_text_file_p = true;
> pph_file = query_pph_include_map (name);
> - if (pph_file != NULL && !cpp_included_before (reader, name,
> input_location))
> + if (pph_file != NULL
> + && pph_is_valid_here (name, loc)
> + && !cpp_included_before (reader, name, input_location))
> {
> /* Hack. We do this to mimic what the non-pph compiler does in
> _cpp_stack_include as our goal is to have identical line_tables. */
> diff --git a/gcc/testsuite/g++.dg/pph/pph.exp
> b/gcc/testsuite/g++.dg/pph/pph.exp
> index 542cf3e..a632365 100644
> --- a/gcc/testsuite/g++.dg/pph/pph.exp
> +++ b/gcc/testsuite/g++.dg/pph/pph.exp
> @@ -38,7 +38,6 @@ exec echo "math.h math.pph" >> pph.map
> exec echo "stdio.h stdio.pph" >> pph.map
> exec echo "stdlib.h stdlib.pph" >> pph.map
> exec echo "string.h string.pph" >> pph.map
> -exec echo "sys/types.h types.pph" >> pph.map
>
> set mapflag -fpph-map=pph.map
>
> diff --git a/gcc/testsuite/g++.dg/pph/y8inc-nmspc.cc
> b/gcc/testsuite/g++.dg/pph/y8inc-nmspc.cc
> index a8784f7..70b209a 100644
> --- a/gcc/testsuite/g++.dg/pph/y8inc-nmspc.cc
> +++ b/gcc/testsuite/g++.dg/pph/y8inc-nmspc.cc
> @@ -1,4 +1,3 @@
> namespace smother {
> -#include "x1struct1.h"
> -// { dg-error "pph file not included at global scope" "" { xfail *-*-* } }
> +#include "x1struct1.h" // { dg-error "PPH file .* not included at global
> scope" "" }
> }
>
> --
> This patch is available for review at http://codereview.appspot.com/4958045
