Improve handling of negative array length

README.rst

@@ -326,8 +326,10 @@ neither you-functions nor defeat-functions may be used.
 
 Preemptive defeat functions
 ---------------------------
+You can use ``preempt`` in defeat functions, but with some restrictions.
+
 The ``preempt`` block is inherently nonlocal, but usually we want at
-least SOME locality when using it.  When ``preempt`` is used directly
+least *some* locality when using it.  When ``preempt`` is used directly
 within a try block, the locality is provided by the try block.  However,
 when used within a defeat function, the intuitive behavior is for it to
 be local to the function's scope rather than to the parent try block, as
@@ -372,8 +374,8 @@ This means that the following code will produce an error:
     }
 
 However, if either the defeat or the preempt is removed, or if the
-function is inlined, or if the code is run with ``--unchecked``, there
-will be no error.
+function is inlined, or if the code is compiled with ``--unchecked``,
+there will be no error.
 
 Command-line arguments
 ----------------------
@@ -460,19 +462,18 @@ the causes of an error than if such errors caused defeat.
 
 Errors can also be produced in user code with ``all_is_broken()``.
 
-Although many operations in HiD are checked and will produce errors, the
-following are undefined behavior:
-
-- Accessing uninitialized strings in dynamically allocated arrays
-- Dynamically allocating an array with negative length (usually this
-  will produce a stack-overflow error, but not necessarily)
+Although many operations in HiD are checked and will produce runtime
+errors, there is some undefined behavior in the case of dynamically
+allocated arrays.  Dynamically allocated arrays are uninitialized, and
+the use of uninitialized strings is undefined behavior.
 
 Additionally, if the ``--unchecked`` flag is passed, all previously
 checked operations become undefined behavior:
 
 - Division or modulo by 0
 - Indexing an array or string out of bounds
 - Stack overflow
+- Dynamically allocating an array with negative length
 - Calling a preemptive defeat function without providing safety
 
 Be aware that HiD's nasal demons can time travel, so undefined behavior

hidc/codegen/generator.py

@@ -796,30 +796,38 @@ def eval_expr(self, r_out: asm.LabelRef, expr: ast.Expression, keep: bool)->asm.
             case ast.ArrayInitializer():
                 length_bubble = yield from self.push_expr(self.r0, expr.length)
                 length = yield from length_bubble.get_fast(self.r0)
+                if not self.unchecked and not expr.type.el_type.byte_sized:
+                    # The byte-sized case will get properly handled by
+                    # stack overflow check, but we need to deal with the
+                    # possibility of integer overflow in get_array_size
+                    # messing up the check.
+                    safe_length = self.add_label('safe_length')
+                    yield asm.Jump(safe_length)
+                    yield asm.Hleu(length, asm.IntLiteral(self.max_length(expr.type.el_type)))
+                    yield from self.goto(stdlib.stack_overflow)
+                    yield asm.Label(safe_length)
                 origin_bubble = self.reserve_word()
                 yield from origin_bubble.value.set(asm.State(self.ap))
                 size = yield from self.get_array_size(self.r0, expr.type.el_type, length)
                 yield asm.Metadata('Array allocation (ArrayInitializer)')
-                no_overflow = self.add_label('no_overflow')
-                yield asm.Jump(no_overflow)
-                yield asm.Sub(self.r1, asm.State(self.fp), asm.State(self.ap))
-
-                # Current static array size is already included in ap
-                cur_static_array = self.stack.static_array_size
-                yield asm.Sub(
-                    self.r1, asm.State(self.r1),
-                    self.checkpoints.add(self.stack.static_size).map(
-                        lambda max_size: max_size - cur_static_array
-                    )
-                )
 
-                # TODO: in the case of word-celled arrays, the length
-                #  might be bad, but the size would overflow and appear
-                #  fine.  Is it worth checking for that?
-                yield asm.Hgeu(asm.State(self.r1), size)
-                yield from self.goto(stdlib.stack_overflow)
+                if not self.unchecked:
+                    no_overflow = self.add_label('no_overflow')
+                    yield asm.Jump(no_overflow)
+                    yield asm.Sub(self.r1, asm.State(self.fp), asm.State(self.ap))
+
+                    # Current static array size is already included in ap
+                    cur_static_array = self.stack.static_array_size
+                    yield asm.Sub(
+                        self.r1, asm.State(self.r1),
+                        self.checkpoints.add(self.stack.static_size).map(
+                            lambda max_size: max_size - cur_static_array
+                        )
+                    )
 
-                yield asm.Label(no_overflow)
+                    yield asm.Hgeu(asm.State(self.r1), size)
+                    yield from self.goto(stdlib.stack_overflow)
+                    yield asm.Label(no_overflow)
 
 
                 yield asm.Add(self.ap, asm.State(self.ap), size)
@@ -968,7 +976,8 @@ def make_global(self, initializer: ast.Expression, const: bool, label_prefix='da
                     return self.add_global_array(
                         const, el_type, label_prefix, length, asm.ZeroDirective(
                             asm.IntLiteral(self.array_size(el_type, length))
-                        )
+                        ),
+                        initializer.length.span
                     )
             case ast.ArrayLiteral():
                 # Using const=True relies on that we don't actually put
@@ -995,7 +1004,8 @@ def make_global(self, initializer: ast.Expression, const: bool, label_prefix='da
 
                 return self.add_global_array(
                     const, initializer.type.el_type,
-                    label_prefix, len(values), directive
+                    label_prefix, len(values), directive,
+                    initializer.span
                 )
         # We treat it as a CodeGenError rather than an internal error if
         # we cannot evaluate the initializer at compile time, implying
@@ -1017,8 +1027,9 @@ def pack_bools(bools, *, bit_size=8):
             result[-1] |= b << bit
         return result
 
-    def add_global_array(self, const, el_type: DataType, label_prefix, length, directive):
-        length = min(length, self.max_length(el_type))
+    def add_global_array(self, const, el_type: DataType, label_prefix, length, directive, span):
+        if length > self.max_length(el_type):
+            raise CodeGenError(f'Array is too large (length: {length})', span)
         label = self.add_label(label_prefix)
         data_dict = self.const_data if const else self.state_data
         access = AccessMode.RC if const else AccessMode.RW
@@ -1431,7 +1442,7 @@ def array_size(self, data_type, length):
     def get_array_size(self, r_out: asm.LabelRef, data_type, length: asm.AssemblyExpression):
         assert isinstance(data_type, DataType)
         if isinstance(length, asm.IntLiteral):
-            return asm.IntLiteral(self.array_size(data_type, length.data))
+            return asm.IntLiteral(self.array_size(data_type, length.data) & self.max_unsigned)
         if data_type == DataType.BOOL:
             yield asm.Add(r_out, length, asm.IntLiteral(7))
             yield asm.Asr(r_out, asm.State(r_out), asm.IntLiteral(3))