poke-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] pkl: fix invalid access to STRUCT_TYPE_FIELD node


From: Jose E. Marchesi
Subject: Re: [PATCH] pkl: fix invalid access to STRUCT_TYPE_FIELD node
Date: Thu, 06 Jul 2023 10:16:10 +0200
User-agent: Gnus/5.13 (Gnus v5.13)

Also, don't we have a similar problem in struct_mapper?

> Hi Mohammad.
> Thanks for the patch.
>
>> Using PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P accessor on a PKL_AST_DECL
>> node happend to not cause any problem till the previous commit, in which,
>> we removed a field and added two new fields.  That change caused
>> PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P to return a non-zero value on 32-bit
>> platforms (i386 and armv7).  The result was a messed up lexical environment.
>>
>> 2023-07-06  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>>
>>      * libpoke/pkl-gen.pks (struct_mapper): Make sure `@field' is of
>>      type `PKL_AST_STRUCT_TYPE_FIELD' before using the field accessor.
>>      (struct_constructor): Likewise.
>> ---
>>  ChangeLog           |  6 ++++++
>>  libpoke/pkl-gen.pks | 15 +++++++++++----
>>  2 files changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/ChangeLog b/ChangeLog
>> index eb347477..200db0a9 100644
>> --- a/ChangeLog
>> +++ b/ChangeLog
>> @@ -1,3 +1,9 @@
>> +2023-07-06  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>> +
>> +    * libpoke/pkl-gen.pks (struct_mapper): Make sure `@field' is of
>> +    type `PKL_AST_STRUCT_TYPE_FIELD' before using the field accessor.
>> +    (struct_constructor): Likewise.
>> +
>>  2023-07-02  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>>  
>>      * libpoke/pkl-ast.h (PKL_AST_DECL_PREV_DECL): New macro.
>> diff --git a/libpoke/pkl-gen.pks b/libpoke/pkl-gen.pks
>> index 0faa98c9..09177156 100644
>> --- a/libpoke/pkl-gen.pks
>> +++ b/libpoke/pkl-gen.pks
>> @@ -1393,7 +1393,10 @@
>>   .c     if (PKL_AST_CODE (@field) != PKL_AST_DECL
>>   .c         || PKL_AST_DECL_KIND (@field) != PKL_AST_DECL_KIND_TYPE)
>>   .c     {
>> - .c       if (!PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P (@field))
>> + .c       if (PKL_AST_CODE (@field) == PKL_AST_STRUCT_TYPE_FIELD
>> + .c           && PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P (@field))
>> + .c         ;
>> + .c       else
>>   .c         i++;
>
> I would find it more clear to use positive logic in the conditional:
>
> if (PKL_AST_CODE (@field) != PKL_AST_STRUCT_TYPE_FIELD
>     || !PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P (@field))
>   i++;
>
>>   .c     }
>>   .c     continue;
>> @@ -1698,10 +1701,9 @@
>>          .label .omitted_field
>>          .label .got_value
>>          .label .constructed_value
>> -        .let @field_initializer = PKL_AST_STRUCT_TYPE_FIELD_INITIALIZER 
>> (@field)
>> -        .let @field_type = PKL_AST_STRUCT_TYPE_FIELD_TYPE (@field)
>>   .c   if (PKL_AST_CODE (@field) != PKL_AST_STRUCT_TYPE_FIELD)
>>   .c   {
>> + .c     assert (PKL_AST_CODE (@field) == PKL_AST_DECL);
>>   .c     /* This is a declaration.  Generate it.  */
>>   .c     PKL_GEN_PUSH_CONTEXT;
>>   .c     PKL_PASS_SUBPASS (@field);
>> @@ -1713,6 +1715,8 @@
>>   .c
>>   .c     continue;
>>   .c   }
>> +        .let @field_initializer = PKL_AST_STRUCT_TYPE_FIELD_INITIALIZER 
>> (@field)
>> +        .let @field_type = PKL_AST_STRUCT_TYPE_FIELD_TYPE (@field)
>>   .c     /* If this is a computed field, ignore it.  */
>>   .c   if (PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P (@field))
>>   .c     continue;
>> @@ -2026,7 +2030,10 @@
>>   .c     if (PKL_AST_CODE (@field) != PKL_AST_DECL
>>   .c         || PKL_AST_DECL_KIND (@field) != PKL_AST_DECL_KIND_TYPE)
>>   .c     {
>> - .c       if (!PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P (@field))
>> + .c       if (PKL_AST_CODE (@field) == PKL_AST_STRUCT_TYPE_FIELD
>> + .c           && PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P (@field))
>> + .c         ;
>> + .c       else
>
> Ditto.
>
>>   .c         i++;
>>   .c     }
>>   .c     continue;
>
> Given that, LGTM.
> OK for both master and maint/poke-3.
>
> Thanks!



reply via email to

[Prev in Thread] Current Thread [Next in Thread]