On Sun, Jan 26, 2014 at 6:14 PM, Matt Turner <matts...@gmail.com> wrote:
> The check was in the wrong place, such that if a shader incorrectly put
> a preprocessor token before the #version declaration, the version would
> be resolved twice, leading to a segmentation fault when attempting to
> redefine the __VERSION__ macro.
>
> #define GL_ARB_sample_shading
> #version 130
> void main() {}
>
> Also, rename glcpp_parser_resolve_version to
> glcpp_parser_resolve_implicit_version to avoid confusion.
> ---
> I couldn't come up with something better to clarify the naming. Feel free
> to suggest something better.
>
> src/glsl/glcpp/glcpp-parse.y | 22 +++++++++++-----------
> src/glsl/glcpp/glcpp.h | 2 +-
> src/glsl/glcpp/pp.c | 2 +-
> 3 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
> index 184e5c2..efcf139 100644
> --- a/src/glsl/glcpp/glcpp-parse.y
> +++ b/src/glsl/glcpp/glcpp-parse.y
> @@ -194,7 +194,7 @@ line:
> ralloc_asprintf_rewrite_tail (&parser->output,
> &parser->output_length, "\n");
> }
> | HASH_LINE {
> - glcpp_parser_resolve_version(parser);
> + glcpp_parser_resolve_implicit_version(parser);
> } pp_tokens NEWLINE {
>
> if (parser->skip_stack == NULL ||
> @@ -254,10 +254,10 @@ define:
>
> control_line:
> HASH_DEFINE {
> - glcpp_parser_resolve_version(parser);
> + glcpp_parser_resolve_implicit_version(parser);
> } define
> | HASH_UNDEF {
> - glcpp_parser_resolve_version(parser);
> + glcpp_parser_resolve_implicit_version(parser);
> } IDENTIFIER NEWLINE {
> macro_t *macro = hash_table_find (parser->defines, $3);
> if (macro) {
> @@ -267,7 +267,7 @@ control_line:
> ralloc_free ($3);
> }
> | HASH_IF {
> - glcpp_parser_resolve_version(parser);
> + glcpp_parser_resolve_implicit_version(parser);
> } conditional_tokens NEWLINE {
> /* Be careful to only evaluate the 'if' expression if
> * we are not skipping. When we are skipping, we
> @@ -299,14 +299,14 @@ control_line:
> _glcpp_parser_skip_stack_push_if (parser, & @1, 0);
> }
> | HASH_IFDEF {
> - glcpp_parser_resolve_version(parser);
> + glcpp_parser_resolve_implicit_version(parser);
> } IDENTIFIER junk NEWLINE {
> macro_t *macro = hash_table_find (parser->defines, $3);
> ralloc_free ($3);
> _glcpp_parser_skip_stack_push_if (parser, & @1, macro !=
> NULL);
> }
> | HASH_IFNDEF {
> - glcpp_parser_resolve_version(parser);
> + glcpp_parser_resolve_implicit_version(parser);
> } IDENTIFIER junk NEWLINE {
> macro_t *macro = hash_table_find (parser->defines, $3);
> ralloc_free ($3);
> @@ -380,7 +380,7 @@ control_line:
> _glcpp_parser_handle_version_declaration(parser, $2, $3,
> true);
> }
> | HASH NEWLINE {
> - glcpp_parser_resolve_version(parser);
> + glcpp_parser_resolve_implicit_version(parser);
> }
> ;
>
> @@ -2024,6 +2024,9 @@ _glcpp_parser_handle_version_declaration(glcpp_parser_t
> *parser, intmax_t versio
> {
> const struct gl_extensions *extensions = parser->extensions;
>
> + if (parser->version_resolved)
> + return;
> +
> parser->version_resolved = true;
Would this lead to the version being stuck at the implicit version
since parser->version_resolved would be set in the implicit case too?
It looks like something like:
#define GL_ARB_sample_shading
#version 150
...might not end up hitting this code:
if (version >= 150)
add_builtin_define(parser, "GL_core_profile", 1);
-Jordan
> add_builtin_define (parser, "__VERSION__", version);
> @@ -2144,11 +2147,8 @@
> _glcpp_parser_handle_version_declaration(glcpp_parser_t *parser, intmax_t
> versio
> #define IMPLICIT_GLSL_VERSION 110
>
> void
> -glcpp_parser_resolve_version(glcpp_parser_t *parser)
> +glcpp_parser_resolve_implicit_version(glcpp_parser_t *parser)
> {
> - if (parser->version_resolved)
> - return;
> -
> _glcpp_parser_handle_version_declaration(parser,
> IMPLICIT_GLSL_VERSION,
> NULL, false);
> }
> diff --git a/src/glsl/glcpp/glcpp.h b/src/glsl/glcpp/glcpp.h
> index 4aa200a..9d85c16 100644
> --- a/src/glsl/glcpp/glcpp.h
> +++ b/src/glsl/glcpp/glcpp.h
> @@ -203,7 +203,7 @@ void
> glcpp_parser_destroy (glcpp_parser_t *parser);
>
> void
> -glcpp_parser_resolve_version(glcpp_parser_t *parser);
> +glcpp_parser_resolve_implicit_version(glcpp_parser_t *parser);
>
> int
> glcpp_preprocess(void *ralloc_ctx, const char **shader, char **info_log,
> diff --git a/src/glsl/glcpp/pp.c b/src/glsl/glcpp/pp.c
> index 637a58f..fc645f7 100644
> --- a/src/glsl/glcpp/pp.c
> +++ b/src/glsl/glcpp/pp.c
> @@ -151,7 +151,7 @@ glcpp_preprocess(void *ralloc_ctx, const char **shader,
> char **info_log,
> if (parser->skip_stack)
> glcpp_error (&parser->skip_stack->loc, parser, "Unterminated
> #if\n");
>
> - glcpp_parser_resolve_version(parser);
> + glcpp_parser_resolve_implicit_version(parser);
>
> ralloc_strcat(info_log, parser->info_log);
>
> --
> 1.8.3.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
