From 7a6cb04b1c6b3a0891bfb49072386d92f49b57dc Mon Sep 17 00:00:00 2001 From: Jose Date: Wed, 29 Apr 2026 12:16:48 +0200 Subject: [PATCH 1/2] Use checked casts in ConvertBasicEnum The signed/unsigned casts were unchecked, so a slicec bug that emitted an out-of-range enumerator value would silently wrap to a valid-looking but wrong C# enum value. Wrap each cast in checked so the failure becomes a loud OverflowException at code-gen time instead of a runtime encode/decode mismatch in user code. The Int64 branch needs special handling for long.MinValue, whose absolute value (2^63) doesn't fit in long. --- src/ZeroC.Slice.Symbols/SymbolConverter.cs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/ZeroC.Slice.Symbols/SymbolConverter.cs b/src/ZeroC.Slice.Symbols/SymbolConverter.cs index 4577e5d339..24e2828963 100644 --- a/src/ZeroC.Slice.Symbols/SymbolConverter.cs +++ b/src/ZeroC.Slice.Symbols/SymbolConverter.cs @@ -318,37 +318,42 @@ private ISymbol ConvertBasicEnum(Compiler.BasicEnum raw, Module module) raw, module, builtin, - (abs, isNegative) => isNegative ? (sbyte)-(long)abs : (sbyte)abs), + (abs, isNegative) => checked((sbyte)(isNegative ? -(long)abs : (long)abs))), BuiltinKind.UInt8 => CreateBasicEnum( raw, module, builtin, - (abs, _) => (byte)abs), + (abs, _) => checked((byte)abs)), BuiltinKind.Int16 => CreateBasicEnum( raw, module, builtin, - (abs, isNegative) => isNegative ? (short)-(long)abs : (short)abs), + (abs, isNegative) => checked((short)(isNegative ? -(long)abs : (long)abs))), BuiltinKind.UInt16 => CreateBasicEnum( raw, module, builtin, - (abs, _) => (ushort)abs), + (abs, _) => checked((ushort)abs)), BuiltinKind.Int32 or BuiltinKind.VarInt32 => CreateBasicEnum( raw, module, builtin, - (abs, isNegative) => isNegative ? (int)-(long)abs : (int)abs), + (abs, isNegative) => checked((int)(isNegative ? -(long)abs : (long)abs))), BuiltinKind.UInt32 or BuiltinKind.VarUInt32 => CreateBasicEnum( raw, module, builtin, - (abs, _) => (uint)abs), + (abs, _) => checked((uint)abs)), BuiltinKind.Int64 or BuiltinKind.VarInt62 => CreateBasicEnum( raw, module, builtin, - (abs, isNegative) => isNegative ? -(long)abs : (long)abs), + // slicec's descriptor format reports each enumerator as (AbsoluteValue: ulong, HasNegativeValue: bool). + // long.MinValue's absolute value is 2^63, which doesn't fit in long — handle it explicitly so the + // checked cast below doesn't throw on a legitimate value. + (abs, isNegative) => isNegative + ? (abs == (ulong)long.MaxValue + 1 ? long.MinValue : checked(-(long)abs)) + : checked((long)abs)), BuiltinKind.UInt64 or BuiltinKind.VarUInt62 => CreateBasicEnum( raw, module, From 9385d7b672a83a692d6f1300ef6f665386d00a0a Mon Sep 17 00:00:00 2001 From: Jose Date: Wed, 29 Apr 2026 15:15:46 +0200 Subject: [PATCH 2/2] Fix sparse-enum misclassification when bounds span underlying range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NeedsHashSetValidation computed contiguity as count - 1 < max - min using INumber arithmetic, which wraps unchecked. For sparse enums whose declared min/max span (or near-span) the underlying type's full range, max - min overflowed to a negative value, the test returned false, and the generator emitted the range-check branch. The resulting "value is >= min and <= max" then accepted every underlying-type value as valid — silently letting invalid wire data become valid enum values. Promote the subtraction to BigInteger so the contiguity test stays correct regardless of underlying-type size or the declared bounds. --- .../BasicEnumGenerator.cs | 9 +++-- .../ZeroC.Slice.Generator.Tests/EnumTests.cs | 35 +++++++++++++++++++ .../EnumTests.slice | 18 ++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/src/ZeroC.Slice.Generator/BasicEnumGenerator.cs b/src/ZeroC.Slice.Generator/BasicEnumGenerator.cs index 68fd7f340d..143c904912 100644 --- a/src/ZeroC.Slice.Generator/BasicEnumGenerator.cs +++ b/src/ZeroC.Slice.Generator/BasicEnumGenerator.cs @@ -226,8 +226,7 @@ private static string GetArticle(string word) => private static bool NeedsHashSetValidation(BasicEnum enumDef) where T : struct, INumber { // If the enumerator count covers the full range of the underlying type, every value is valid - // and no validation is needed. This also prevents overflow when computing max - min below - // for small types like sbyte where MaxValue - MinValue (127 - (-128) = 255) overflows. + // and no validation is needed. int? bitSize = enumDef.Underlying.Kind switch { BuiltinKind.Int8 or BuiltinKind.UInt8 => 8, @@ -244,6 +243,10 @@ private static bool NeedsHashSetValidation(BasicEnum enumDef) where T : st T min = enumDef.Enumerators.Min(e => e.Value); T max = enumDef.Enumerators.Max(e => e.Value); - return T.CreateChecked(enumDef.Enumerators.Count - 1) < max - min; + // Promote to BigInteger so the subtraction doesn't wrap when min/max span (or near-span) the + // underlying type's full range — otherwise a sparse enum like `int8 { Min = -128, Max = 127 }` + // is misclassified as contiguous and the emitted range check accepts every underlying value. + BigInteger span = BigInteger.CreateChecked(max) - BigInteger.CreateChecked(min); + return enumDef.Enumerators.Count - 1 < span; } } diff --git a/tests/ZeroC.Slice.Generator.Tests/EnumTests.cs b/tests/ZeroC.Slice.Generator.Tests/EnumTests.cs index fe96474e2c..7293b8c059 100644 --- a/tests/ZeroC.Slice.Generator.Tests/EnumTests.cs +++ b/tests/ZeroC.Slice.Generator.Tests/EnumTests.cs @@ -46,6 +46,41 @@ public void As_enum_for_an_enum_with_contiguous_enumerators(int value, MyEnum ex public void As_enum_for_an_enum_with_contiguous_enumerators_fails_for_invalid_values(int value) => Assert.That(() => value.AsMyEnum(), Throws.TypeOf()); + // Boundary enums: declared min/max span the full underlying range. The valid enumerators decode, + // and intermediate values that are not declared must be rejected. + [TestCase((sbyte)-128, MyInt8BoundaryEnum.Min)] + [TestCase((sbyte)127, MyInt8BoundaryEnum.Max)] + public void As_enum_int8_boundary_accepts_declared_values(sbyte value, MyInt8BoundaryEnum expected) => + Assert.That(value.AsMyInt8BoundaryEnum(), Is.EqualTo(expected)); + + [TestCase((sbyte)-127)] + [TestCase((sbyte)0)] + [TestCase((sbyte)126)] + public void As_enum_int8_boundary_rejects_undeclared_values(sbyte value) => + Assert.That(() => value.AsMyInt8BoundaryEnum(), Throws.TypeOf()); + + [TestCase(int.MinValue, MyInt32BoundaryEnum.Min)] + [TestCase(int.MaxValue, MyInt32BoundaryEnum.Max)] + public void As_enum_int32_boundary_accepts_declared_values(int value, MyInt32BoundaryEnum expected) => + Assert.That(value.AsMyInt32BoundaryEnum(), Is.EqualTo(expected)); + + [TestCase(int.MinValue + 1)] + [TestCase(0)] + [TestCase(int.MaxValue - 1)] + public void As_enum_int32_boundary_rejects_undeclared_values(int value) => + Assert.That(() => value.AsMyInt32BoundaryEnum(), Throws.TypeOf()); + + [TestCase(long.MinValue, MyInt64BoundaryEnum.Min)] + [TestCase(long.MaxValue, MyInt64BoundaryEnum.Max)] + public void As_enum_int64_boundary_accepts_declared_values(long value, MyInt64BoundaryEnum expected) => + Assert.That(value.AsMyInt64BoundaryEnum(), Is.EqualTo(expected)); + + [TestCase(long.MinValue + 1)] + [TestCase(0L)] + [TestCase(long.MaxValue - 1)] + public void As_enum_int64_boundary_rejects_undeclared_values(long value) => + Assert.That(() => value.AsMyInt64BoundaryEnum(), Throws.TypeOf()); + [TestCase(sizeof(MyFixedSizeEnum), sizeof(short))] [TestCase(sizeof(MyUncheckedEnum), sizeof(uint))] public void Enum_has_the_expected_size(int size, int expectedSize) => diff --git a/tests/ZeroC.Slice.Generator.Tests/EnumTests.slice b/tests/ZeroC.Slice.Generator.Tests/EnumTests.slice index 2a39d4cb0e..207e29e9d8 100644 --- a/tests/ZeroC.Slice.Generator.Tests/EnumTests.slice +++ b/tests/ZeroC.Slice.Generator.Tests/EnumTests.slice @@ -32,3 +32,21 @@ unchecked enum MyUncheckedEnum : uint32 { } unchecked enum MyVarInt62Alias : varint62 {} + +// Sparse enums whose declared min/max span the full underlying range. Exercises both the BigInteger-based +// contiguity test (must classify these as sparse → HashSet validation) and the checked casts in the +// symbol converter (boundary values must round-trip without overflow). +enum MyInt8BoundaryEnum : int8 { + Min = -128 + Max = 127 +} + +enum MyInt32BoundaryEnum : int32 { + Min = -2147483648 + Max = 2147483647 +} + +enum MyInt64BoundaryEnum : int64 { + Min = -9223372036854775808 + Max = 9223372036854775807 +}