[Mesa-dev] glsl: Fix glcpp to properly lex entire "preprocessing numbers"

Submitted by Carl Worth on June 12, 2014, 10:13 p.m.

Details

Message ID 1402611194-30954-1-git-send-email-cworth@cworth.org
State New
Headers show

Not browsing as part of any series.

Commit Message

Carl Worth June 12, 2014, 10:13 p.m.
The preprocessor defines a notions of a "preprocessing number" that
starts with either a digit or a decimal point, and continues with zero
or more of digits, decimal points, identifier characters, or the sign
symbols, ('-' and '+').

Prior to this change, preprocessing numbers were lexed as some
combination of OTHER and IDENTIFIER tokens. This had the problem of
causing undesired macro expansion in some cases.

We add tests to ensure that the undesired macro expansion does not
happen in cases such as:

	#define e +1
        #define xyz -2

        int n = 1e;
        int p = 1xyz;

In either case these macro definitions have no effect after this
change, so that the numeric literals, (whether valid or not), will be
passed on as-is from the preprocessor to the compiler proper.

This fixes at least the following Khronos GLES3 CTS test:

preprocessor.basic.correct_phases_fragment
---
 src/glsl/glcpp/glcpp-lex.l                                | 6 ++++++
 src/glsl/glcpp/tests/124-preprocessing-numbers.c          | 8 ++++++++
 src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected | 9 +++++++++
 3 files changed, 23 insertions(+)
 create mode 100644 src/glsl/glcpp/tests/124-preprocessing-numbers.c
 create mode 100644 src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected

Patch hide | download patch | download mbox

diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
index d5fb087..4dbaa9e 100644
--- a/src/glsl/glcpp/glcpp-lex.l
+++ b/src/glsl/glcpp/glcpp-lex.l
@@ -91,6 +91,7 @@  DIGITS			[0-9][0-9]*
 DECIMAL_INTEGER		[1-9][0-9]*[uU]?
 OCTAL_INTEGER		0[0-7]*[uU]?
 HEXADECIMAL_INTEGER	0[xX][0-9a-fA-F]+[uU]?
+PP_NUMBER		[0-9.][-+._a-zA-Z0-9]*
 
 %%
 
@@ -339,6 +340,11 @@  HEXADECIMAL_INTEGER	0[xX][0-9a-fA-F]+[uU]?
 	return OTHER;
 }
 
+{PP_NUMBER} {
+	yylval->str = ralloc_strdup (yyextra, yytext);
+	return OTHER;
+}
+
 {HSPACE} {
 	if (yyextra->space_tokens) {
 		return SPACE;
diff --git a/src/glsl/glcpp/tests/124-preprocessing-numbers.c b/src/glsl/glcpp/tests/124-preprocessing-numbers.c
new file mode 100644
index 0000000..3743c69
--- /dev/null
+++ b/src/glsl/glcpp/tests/124-preprocessing-numbers.c
@@ -0,0 +1,8 @@ 
+#define e +1
+#define xyz -2
+
+/* The following are "preprocessing numbers" and should not trigger macro
+ * expansion. */
+int n = 1e;
+int p = 1xyz;
+
diff --git a/src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected b/src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected
new file mode 100644
index 0000000..951581e
--- /dev/null
+++ b/src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected
@@ -0,0 +1,9 @@ 
+
+
+
+ 
+
+int n = 1e;
+int p = 1xyz;
+
+

Comments

On Thu, Jun 12, 2014 at 3:13 PM, Carl Worth <cworth@cworth.org> wrote:
>
> The preprocessor defines a notions of a "preprocessing number" that
> starts with either a digit or a decimal point, and continues with zero
> or more of digits, decimal points, identifier characters, or the sign
> symbols, ('-' and '+').
>
> Prior to this change, preprocessing numbers were lexed as some
> combination of OTHER and IDENTIFIER tokens. This had the problem of
> causing undesired macro expansion in some cases.
>
> We add tests to ensure that the undesired macro expansion does not
> happen in cases such as:
>
>         #define e +1
>         #define xyz -2
>
>         int n = 1e;
>         int p = 1xyz;
>
> In either case these macro definitions have no effect after this
> change, so that the numeric literals, (whether valid or not), will be
> passed on as-is from the preprocessor to the compiler proper.
>
> This fixes at least the following Khronos GLES3 CTS test:
>
> preprocessor.basic.correct_phases_fragment
> ---
>  src/glsl/glcpp/glcpp-lex.l                                | 6 ++++++
>  src/glsl/glcpp/tests/124-preprocessing-numbers.c          | 8 ++++++++
>  src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected | 9 +++++++++
>  3 files changed, 23 insertions(+)
>  create mode 100644 src/glsl/glcpp/tests/124-preprocessing-numbers.c
>  create mode 100644 src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected
>
> diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
> index d5fb087..4dbaa9e 100644
> --- a/src/glsl/glcpp/glcpp-lex.l
> +++ b/src/glsl/glcpp/glcpp-lex.l
> @@ -91,6 +91,7 @@ DIGITS                        [0-9][0-9]*
>  DECIMAL_INTEGER                [1-9][0-9]*[uU]?
>  OCTAL_INTEGER          0[0-7]*[uU]?
>  HEXADECIMAL_INTEGER    0[xX][0-9a-fA-F]+[uU]?
> +PP_NUMBER              [0-9.][-+._a-zA-Z0-9]*
>
>  %%
>
> @@ -339,6 +340,11 @@ HEXADECIMAL_INTEGER        0[xX][0-9a-fA-F]+[uU]?
>         return OTHER;
>  }
>
> +{PP_NUMBER} {
> +       yylval->str = ralloc_strdup (yyextra, yytext);
> +       return OTHER;
> +}
> +
>  {HSPACE} {
>         if (yyextra->space_tokens) {
>                 return SPACE;
> diff --git a/src/glsl/glcpp/tests/124-preprocessing-numbers.c b/src/glsl/glcpp/tests/124-preprocessing-numbers.c
> new file mode 100644
> index 0000000..3743c69
> --- /dev/null
> +++ b/src/glsl/glcpp/tests/124-preprocessing-numbers.c
> @@ -0,0 +1,8 @@
> +#define e +1
> +#define xyz -2
> +
> +/* The following are "preprocessing numbers" and should not trigger macro
> + * expansion. */
> +int n = 1e;
> +int p = 1xyz;
> +
> diff --git a/src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected b/src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected
> new file mode 100644
> index 0000000..951581e
> --- /dev/null
> +++ b/src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected
> @@ -0,0 +1,9 @@
> +
> +
> +
> +
> +
> +int n = 1e;
> +int p = 1xyz;
> +
> +
> --
> 2.0.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
On Thu, Jun 12, 2014 at 5:18 PM, Anuj Phogat <anuj.phogat@gmail.com> wrote:
> On Thu, Jun 12, 2014 at 3:13 PM, Carl Worth <cworth@cworth.org> wrote:
>>
>> The preprocessor defines a notions of a "preprocessing number" that
>> starts with either a digit or a decimal point, and continues with zero
>> or more of digits, decimal points, identifier characters, or the sign
>> symbols, ('-' and '+').
>>
>> Prior to this change, preprocessing numbers were lexed as some
>> combination of OTHER and IDENTIFIER tokens. This had the problem of
>> causing undesired macro expansion in some cases.
>>
>> We add tests to ensure that the undesired macro expansion does not
>> happen in cases such as:
>>
>>         #define e +1
>>         #define xyz -2
>>
>>         int n = 1e;
>>         int p = 1xyz;
>>
>> In either case these macro definitions have no effect after this
>> change, so that the numeric literals, (whether valid or not), will be
>> passed on as-is from the preprocessor to the compiler proper.
>>
>> This fixes at least the following Khronos GLES3 CTS test:
>>
>> preprocessor.basic.correct_phases_fragment
>> ---
>>  src/glsl/glcpp/glcpp-lex.l                                | 6 ++++++
>>  src/glsl/glcpp/tests/124-preprocessing-numbers.c          | 8 ++++++++
>>  src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected | 9 +++++++++
>>  3 files changed, 23 insertions(+)
>>  create mode 100644 src/glsl/glcpp/tests/124-preprocessing-numbers.c
>>  create mode 100644 src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected
>>
>> diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
>> index d5fb087..4dbaa9e 100644
>> --- a/src/glsl/glcpp/glcpp-lex.l
>> +++ b/src/glsl/glcpp/glcpp-lex.l
>> @@ -91,6 +91,7 @@ DIGITS                        [0-9][0-9]*
>>  DECIMAL_INTEGER                [1-9][0-9]*[uU]?
>>  OCTAL_INTEGER          0[0-7]*[uU]?
>>  HEXADECIMAL_INTEGER    0[xX][0-9a-fA-F]+[uU]?
>> +PP_NUMBER              [0-9.][-+._a-zA-Z0-9]*
It causes '2.0-MACRO' to incorrectly lexed as a preprocessing number.
Around 30 GLES3 CTS tests failed with this patch.

I think using below definition of preprocessing numbers should work fine:
PP_NUMBER                   \.?[0-9]([._a-zA-Z0-9]|[eEpP][+-])*

Used below link for definition of a preprocessing number:
https://gcc.gnu.org/onlinedocs/cpp/Tokenization.html

>>
>>  %%
>>
>> @@ -339,6 +340,11 @@ HEXADECIMAL_INTEGER        0[xX][0-9a-fA-F]+[uU]?
>>         return OTHER;
>>  }
>>
>> +{PP_NUMBER} {
>> +       yylval->str = ralloc_strdup (yyextra, yytext);
>> +       return OTHER;
>> +}
>> +
>>  {HSPACE} {
>>         if (yyextra->space_tokens) {
>>                 return SPACE;
>> diff --git a/src/glsl/glcpp/tests/124-preprocessing-numbers.c b/src/glsl/glcpp/tests/124-preprocessing-numbers.c
>> new file mode 100644
>> index 0000000..3743c69
>> --- /dev/null
>> +++ b/src/glsl/glcpp/tests/124-preprocessing-numbers.c
>> @@ -0,0 +1,8 @@
>> +#define e +1
>> +#define xyz -2
>> +
>> +/* The following are "preprocessing numbers" and should not trigger macro
>> + * expansion. */
>> +int n = 1e;
>> +int p = 1xyz;
>> +
>> diff --git a/src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected b/src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected
>> new file mode 100644
>> index 0000000..951581e
>> --- /dev/null
>> +++ b/src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected
>> @@ -0,0 +1,9 @@
>> +
>> +
>> +
>> +
>> +
>> +int n = 1e;
>> +int p = 1xyz;
>> +
>> +
>> --
>> 2.0.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>