[#640] Disallow offsets to be used in expressions

ndsev · Aug 21, 2024 · d3cf197 · d3cf197
1 parent b1c49e9
commit d3cf197
Show file tree

Hide file tree

Showing 37 changed files with 470 additions and 13 deletions.
diff --git a/compiler/core/antlr/ZserioParser.g4 b/compiler/core/antlr/ZserioParser.g4
@@ -57,7 +57,7 @@ symbolDefinition
 // CONST
 
 constDefinition
-    :   CONST typeInstantiation id ASSIGN expression SEMICOLON
+    :   CONST typeInstantiation id ASSIGN expression SEMICOLON // constant expression
     ;
 
 
@@ -107,7 +107,7 @@ structureFieldDefinition
     ;
 
 fieldAlignment
-    :   ALIGN LPAREN expression RPAREN COLON // integer expression
+    :   ALIGN LPAREN expression RPAREN COLON // constant integer expression
     ;
 
 fieldOffset
@@ -191,7 +191,7 @@ enumDeclaration
     ;
 
 enumItem
-    :   (DEPRECATED | REMOVED)? id (ASSIGN expression)?
+    :   (DEPRECATED | REMOVED)? id (ASSIGN expression)? // constant integer expression
     ;
 
 
@@ -206,7 +206,7 @@ bitmaskDeclaration
     ;
 
 bitmaskValue
-    :    id (ASSIGN expression)?
+    :    id (ASSIGN expression)? // constant integer expression
     ;
 
 

diff --git a/compiler/core/src/zserio/ast/ArrayInstantiation.java b/compiler/core/src/zserio/ast/ArrayInstantiation.java
@@ -160,6 +160,8 @@ void check(WarningsConfig warningsConfig, ZserioTemplatableType currentTemplateI
         // check length expression
         if (lengthExpression != null)
         {
+            ExpressionUtil.checkOffsetFields(lengthExpression);
+
             if (lengthExpression.getExprType() != Expression.ExpressionType.INTEGER)
             {
                 throw new ParserException(

diff --git a/compiler/core/src/zserio/ast/ChoiceType.java b/compiler/core/src/zserio/ast/ChoiceType.java
@@ -177,7 +177,7 @@ void check(WarningsConfig warningsConfig)
         checkExtendedFields();
 
         isChoiceDefaultUnreachable = checkUnreachableDefault();
-        checkSelectorType();
+        checkSelectorExpression();
         checkCaseTypes();
         checkEnumerationCases(warningsConfig);
         checkBitmaskCases();
@@ -269,8 +269,10 @@ private int getNumCases()
         return numCases;
     }
 
-    private void checkSelectorType()
+    private void checkSelectorExpression()
     {
+        ExpressionUtil.checkOffsetFields(selectorExpression);
+
         final Expression.ExpressionType selectorExpressionType = selectorExpression.getExprType();
         if (selectorExpressionType != Expression.ExpressionType.INTEGER &&
                 selectorExpressionType != Expression.ExpressionType.BOOLEAN &&

diff --git a/compiler/core/src/zserio/ast/DynamicBitFieldInstantiation.java b/compiler/core/src/zserio/ast/DynamicBitFieldInstantiation.java
@@ -3,6 +3,8 @@
 import java.math.BigInteger;
 import java.util.List;
 
+import zserio.tools.WarningsConfig;
+
 /**
  * AST node for dynamic bit field type instantiation.
  */
@@ -160,6 +162,12 @@ void evaluate()
         }
     }
 
+    @Override
+    void check(WarningsConfig config, ZserioTemplatableType currentTemplateInstantiation)
+    {
+        ExpressionUtil.checkOffsetFields(lengthExpression);
+    }
+
     private final Expression lengthExpression;
 
     private int maxBitSize;

diff --git a/compiler/core/src/zserio/ast/ExpressionUtil.java b/compiler/core/src/zserio/ast/ExpressionUtil.java
@@ -2,6 +2,7 @@
 
 import java.math.BigInteger;
 import java.util.Locale;
+import java.util.Set;
 
 /**
  * This class implements various utilities on Expression type.
@@ -39,7 +40,7 @@ static void checkExpressionType(Expression expression, TypeReference typeReferen
      * Checks if integer expression is within the range of Zserio type.
      *
      * @param expression    Expression to use for checking.
-     * @param instantiation Type intantiation to use for checking.
+     * @param instantiation Type instantiation to use for checking.
      * @param ownerName     Name of Zserio type which owns the given expression.
      *
      * @throws ParserException Throws if integer expression exceeds the bounds of its type.
@@ -65,6 +66,31 @@ static void checkIntegerExpressionRange(
         }
     }
 
+    /**
+     * Checks if the expression contains any field which is used as an offset and throws an exception in that
+     * case.
+     *
+     * @param expression Expression to use for checking.
+     * @throws ParserException Throws if the expression contains any field which is used as an offset.
+     */
+    static void checkOffsetFields(Expression expression)
+    {
+        final Set<Field> referencedFields = expression.getReferencedSymbolObjects(Field.class);
+        for (Field referencedField : referencedFields)
+        {
+            if (referencedField.getUsedAsOffsetLocation() != null)
+            {
+                final ParserStackedException exception = new ParserStackedException(
+                        expression.getLocation(), "Fields used as offsets cannot be used in expressions!");
+                exception.pushMessage(referencedField.getUsedAsOffsetLocation(),
+                        "    Field '" + referencedField.getName() + "' used as an offset here!");
+                exception.pushMessage(referencedField.getLocation(),
+                        "    Field '" + referencedField.getName() + "' defined here!");
+                throw exception;
+            }
+        }
+    }
+
     private static void checkExpressionType(
             Expression expression, ZserioType type, TypeInstantiation instantiation)
     {

diff --git a/compiler/core/src/zserio/ast/Field.java b/compiler/core/src/zserio/ast/Field.java
@@ -230,7 +230,7 @@ public boolean isOptional()
      */
     public boolean isPackable()
     {
-        if (usedAsOffset)
+        if (usedAsOffsetLocation != null)
             return false;
 
         ZserioType fieldBaseType = typeInstantiation.getBaseType();
@@ -261,7 +261,7 @@ void evaluate()
                     throw new ParserException(offsetExpr, "Packed array cannot be used as offset array!");
                 }
 
-                referencedField.usedAsOffset = true;
+                referencedField.usedAsOffsetLocation = offsetExpr.getLocation();
             }
         }
     }
@@ -304,6 +304,8 @@ void check(Package pkg, WarningsConfig warningsConfig)
         // check optional expression type
         if (optionalClauseExpr != null)
         {
+            ExpressionUtil.checkOffsetFields(optionalClauseExpr);
+
             if (optionalClauseExpr.getExprType() != Expression.ExpressionType.BOOLEAN)
                 throw new ParserException(optionalClauseExpr,
                         "Optional expression for field '" + getName() + "' is not boolean!");
@@ -312,6 +314,8 @@ void check(Package pkg, WarningsConfig warningsConfig)
         // check constraint expression type
         if (constraintExpr != null)
         {
+            ExpressionUtil.checkOffsetFields(constraintExpr);
+
             if (constraintExpr.getExprType() != Expression.ExpressionType.BOOLEAN)
                 throw new ParserException(
                         constraintExpr, "Constraint expression for field '" + getName() + "' is not boolean!");
@@ -321,6 +325,17 @@ void check(Package pkg, WarningsConfig warningsConfig)
         checkOptionalReferences(warningsConfig);
     }
 
+    /**
+     * Returns location where the field is used as an offset.
+     * Can be called after evaluate phase!
+     *
+     * @return Location where the field is used in an offset expression, null otherwise.
+     */
+    AstLocation getUsedAsOffsetLocation()
+    {
+        return usedAsOffsetLocation;
+    }
+
     /**
      * Instantiate the field.
      *
@@ -584,7 +599,7 @@ private Field(AstLocation location, boolean isExtended, TypeInstantiation typeIn
         this.isVirtual = isVirtual;
         this.sqlConstraint = sqlConstraint;
 
-        this.usedAsOffset = false;
+        this.usedAsOffsetLocation = null;
     }
 
     private final boolean isExtended;
@@ -602,5 +617,5 @@ private Field(AstLocation location, boolean isExtended, TypeInstantiation typeIn
     private final boolean isVirtual;
     private final SqlConstraint sqlConstraint;
 
-    private boolean usedAsOffset;
+    private AstLocation usedAsOffsetLocation;
 }
diff --git a/compiler/core/src/zserio/ast/Function.java b/compiler/core/src/zserio/ast/Function.java
@@ -80,6 +80,8 @@ public Expression getResultExpression()
      */
     void check(WarningsConfig warningsConfig)
     {
+        ExpressionUtil.checkOffsetFields(resultExpression);
+
         // check result expression type
         ExpressionUtil.checkExpressionType(resultExpression, returnTypeReference);
 

diff --git a/compiler/core/src/zserio/ast/ParameterizedTypeInstantiation.java b/compiler/core/src/zserio/ast/ParameterizedTypeInstantiation.java
@@ -188,6 +188,8 @@ void check(WarningsConfig warningsConfig, ZserioTemplatableType currentTemplateI
             final Expression argumentExpression = instantiatedParameter.getArgumentExpression();
             if (!argumentExpression.isExplicitVariable())
             {
+                ExpressionUtil.checkOffsetFields(argumentExpression);
+
                 final TypeReference parameterTypeReference =
                         instantiatedParameter.getParameter().getTypeReference();
                 ExpressionUtil.checkExpressionType(argumentExpression, parameterTypeReference);

diff --git a/doc/ZserioLanguageOverview.md b/doc/ZserioLanguageOverview.md
@@ -812,6 +812,10 @@ byteOffset:
 In the code example above, if the member is optional (`hasOptional == false`) then no offset will be checked
 and the size will be 65 bits.
 
+> Fields which are used as offsets cannot be used in [expressions](#expressions).
+
+> Each field can be used only once as an offset.
+
 ### Indexed Offsets
 
 When all elements in an array should have offsets, a special notation can be used:
@@ -847,6 +851,10 @@ offsets[@index]: // implies align(8) before each data[i]
 };
 ```
 
+> Fields which are used as indexed offsets cannot be used in [expressions](#expressions).
+
+> Each field can be used only once as an indexed offset.
+
 [top](#language-guide)
 
 ## Expressions

diff --git a/test/errors/array_types_error/build.xml b/test/errors/array_types_error/build.xml
@@ -13,6 +13,10 @@
         <testGenClean testName="array_types_error"/>
         <testGen testName="array_types_error" zsFile="array_length_field_not_available_error.zs"
             ignoreErrors="true"/>
+        <testGen testName="array_types_error" zsFile="array_length_field_used_as_indexed_offset_error.zs"
+            ignoreErrors="true"/>
+        <testGen testName="array_types_error" zsFile="array_length_field_used_as_offset_error.zs"
+            ignoreErrors="true"/>
         <testGen testName="array_types_error" zsFile="deprecated_implicit_array_error.zs" ignoreErrors="true"/>
         <testGen testName="array_types_error" zsFile="implicit_array_bitfield_with_wrong_length_error.zs"
             ignoreErrors="true">

diff --git a/test/errors/array_types_error/java/array_types_error/ArrayTypesErrorTest.java b/test/errors/array_types_error/java/array_types_error/ArrayTypesErrorTest.java
@@ -25,14 +25,39 @@ public void arrayLengthNotAvailable()
         assertTrue(zserioErrors.isPresent(error));
     }
 
+    @Test
+    public void arrayLengthFieldUsedAsIndexedOffset()
+    {
+        final String errors[] = {
+                "array_length_field_used_as_indexed_offset_error.zs:5:12:     Field 'offsets' defined here!",
+                "array_length_field_used_as_indexed_offset_error.zs:16:1: "
+                        + "    Field 'offsets' used as an offset here!",
+                "array_length_field_used_as_indexed_offset_error.zs:10:18: "
+                        + "Fields used as offsets cannot be used in expressions!",
+        };
+        assertTrue(zserioErrors.isPresent(errors));
+    }
+
+    @Test
+    public void arrayLengthFieldUsedAsOffset()
+    {
+        final String errors[] = {
+                "array_length_field_used_as_offset_error.zs:5:12:     Field 'offset' defined here!",
+                "array_length_field_used_as_offset_error.zs:6:1:     Field 'offset' used as an offset here!",
+                "array_length_field_used_as_offset_error.zs:7:19: "
+                        + "Fields used as offsets cannot be used in expressions!",
+        };
+        assertTrue(zserioErrors.isPresent(errors));
+    }
+
     @Test
     public void deprecatedImplicitArray()
     {
         final String errors[] = {
                 "deprecated_implicit_array_error.zs:5:5: For strong compatibility reason, please consider "
                         + "to use command line option '-allowImplicitArrays'.",
-                "deprecated_implicit_array_error.zs:5:5: Implicit arrays are deprecated and will be removed from "
-                        + "the language!"};
+                "deprecated_implicit_array_error.zs:5:5: "
+                        + "Implicit arrays are deprecated and will be removed from the language!"};
         assertTrue(zserioErrors.isPresent(errors));
     }
 

diff --git a/test/errors/array_types_error/zs/array_length_field_used_as_indexed_offset_error.zs b/test/errors/array_types_error/zs/array_length_field_used_as_indexed_offset_error.zs
@@ -0,0 +1,18 @@
+package array_length_field_used_as_indexed_offset_error;
+
+struct Holder
+{
+    uint32 offsets[];
+};
+
+struct Parameterized(Holder holder)
+{
+    uint32 array[holder.offsets[0]];
+};
+
+struct Container
+{
+    Holder holder;
+holder.offsets[@index]:
+    Parameterized(holder) parameterized[];
+};
diff --git a/test/errors/array_types_error/zs/array_length_field_used_as_offset_error.zs b/test/errors/array_types_error/zs/array_length_field_used_as_offset_error.zs
@@ -0,0 +1,8 @@
+package array_length_field_used_as_offset_error;
+
+struct Container
+{
+    uint32 offset;
+offset:
+    string fields[offset];
+};
diff --git a/test/errors/builtin_types_error/build.xml b/test/errors/builtin_types_error/build.xml
@@ -17,6 +17,10 @@
             ignoreErrors="true"/>
         <testGen testName="builtin_types_error" zsFile="bitfield_length_field_not_available_error.zs"
             ignoreErrors="true"/>
+        <testGen testName="builtin_types_error" zsFile="bitfield_length_field_used_as_indexed_offset_error.zs"
+            ignoreErrors="true"/>
+        <testGen testName="builtin_types_error" zsFile="bitfield_length_field_used_as_offset_error.zs"
+            ignoreErrors="true"/>
         <testGen testName="builtin_types_error" zsFile="bitfield_unknown_length_error.zs" ignoreErrors="true"/>
         <testGen testName="builtin_types_error" zsFile="bitfield_without_arg_error.zs" ignoreErrors="true"/>
         <testGen testName="builtin_types_error" zsFile="bitfield0_error.zs" ignoreErrors="true"/>

diff --git a/test/errors/builtin_types_error/java/builtin_types_error/BuiltInTypesErrorTest.java b/test/errors/builtin_types_error/java/builtin_types_error/BuiltInTypesErrorTest.java
@@ -41,6 +41,31 @@ public void bitfieldLengthFieldNotAvailable()
         assertTrue(zserioErrors.isPresent(error));
     }
 
+    @Test
+    public void bitfieldLengthFieldUsedAsIndexedOffset()
+    {
+        final String errors[] = {
+                "bitfield_length_field_used_as_indexed_offset_error.zs:5:12:     Field 'offsets' defined here!",
+                "bitfield_length_field_used_as_indexed_offset_error.zs:6:1: "
+                        + "    Field 'offsets' used as an offset here!",
+                "bitfield_length_field_used_as_indexed_offset_error.zs:8:9: "
+                        + "Fields used as offsets cannot be used in expressions!",
+        };
+        assertTrue(zserioErrors.isPresent(errors));
+    }
+
+    @Test
+    public void bitfieldLengthFieldUsedAsOffset()
+    {
+        final String errors[] = {
+                "bitfield_length_field_used_as_offset_error.zs:5:12:     Field 'offset' defined here!",
+                "bitfield_length_field_used_as_offset_error.zs:6:1:     Field 'offset' used as an offset here!",
+                "bitfield_length_field_used_as_offset_error.zs:8:9: "
+                        + "Fields used as offsets cannot be used in expressions!",
+        };
+        assertTrue(zserioErrors.isPresent(errors));
+    }
+
     @Test
     public void bitfieldUnknownLength()
     {

diff --git a/test/errors/builtin_types_error/zs/bitfield_length_field_used_as_indexed_offset_error.zs b/test/errors/builtin_types_error/zs/bitfield_length_field_used_as_indexed_offset_error.zs
@@ -0,0 +1,9 @@
+package bitfield_length_field_used_as_indexed_offset_error;
+
+struct Container
+{
+    uint32 offsets[];
+offsets[@index]:
+    string fields[];
+    bit<offsets[0]> bitField;
+};
diff --git a/test/errors/builtin_types_error/zs/bitfield_length_field_used_as_offset_error.zs b/test/errors/builtin_types_error/zs/bitfield_length_field_used_as_offset_error.zs
@@ -0,0 +1,9 @@
+package bitfield_length_field_used_as_offset_error;
+
+struct Container
+{
+    uint32 offset;
+offset:
+    string field;
+    bit<offset> bitField;
+};