From 760773b5bef161dcb26ffb19c3e292b1e240cf2d Mon Sep 17 00:00:00 2001 From: neuecc Date: Tue, 3 Jan 2023 02:48:43 +0900 Subject: [PATCH] improve unmanaged struct validation --- sandbox/SandboxConsoleApp/Program.cs | 67 +++++++++++++ sandbox/SandboxNet6/Program.cs | 74 ++++++++++++++ .../DiagnosticDescriptors.cs | 16 +++ .../MemoryPackGenerator.Parser.cs | 99 +++++++++++++++++-- src/MemoryPack.Generator/ReferenceSymbols.cs | 9 ++ .../GeneratorDiagnosticsTest.TypeScript.cs | 4 +- .../GeneratorDiagnosticsTest.cs | 51 ++++++++++ .../Utils/CSharpGeneratorRunner.cs | 12 ++- 8 files changed, 317 insertions(+), 15 deletions(-) diff --git a/sandbox/SandboxConsoleApp/Program.cs b/sandbox/SandboxConsoleApp/Program.cs index b973c809..f030bd2d 100644 --- a/sandbox/SandboxConsoleApp/Program.cs +++ b/sandbox/SandboxConsoleApp/Program.cs @@ -631,3 +631,70 @@ public partial class Sample [MemoryPackInclude] int privateProperty2 { get; set; } } + + +public struct DateTimeParamDefault +{ + public DateTimeOffset DateTime; // short offset(2+padding) + dateTime/ulong(8) = 16 + public long Timestamp; // 8 + public bool IsItInSeconds; // 1(+padding7) = 8 +} + +[StructLayout(LayoutKind.Sequential)] +[MemoryPackable] +public partial struct TesTest +{ + public ValueTuple VTI; + public MyMessageHeader MyMsgHead; + public bool IsItInSeconds; // 1(+padding7) = 8 +} + +[StructLayout(LayoutKind.Sequential)] +[MemoryPackable] +public partial struct DateTimeParamSequential +{ + public DateTimeOffset DateTime; // short offset(2+padding) + dateTime/ulong(8) = 16 + public long Timestamp; // 8 + public bool IsItInSeconds; // 1(+padding7) = 8 +} + +[StructLayout(LayoutKind.Auto)] +[MemoryPackable] +public partial struct DateTimeParamAuto +{ + public DateTimeOffset DateTime; // short offset(2+padding) + dateTime/ulong(8) = 16 + public long Timestamp; // 8 + public bool IsItInSeconds; // 1(+padding7) = 8 +} + +[StructLayout(LayoutKind.Explicit, Size = 25)] +[MemoryPackable] +public partial struct DateTimeParamExplicit +{ + [FieldOffset(9)] + public DateTimeOffset DateTime; + [FieldOffset(1)] + public long Timestamp; // 8 + [FieldOffset(0)] + public bool IsItInSeconds; // 1 +} + +[StructLayout(LayoutKind.Auto)] +public struct MyMessageHeader +{ +} + +[MemoryPackable] +public partial struct HogeEEE +{ + public int X; + public int Y; + + // [MemoryPackConstructor] + public HogeEEE(int x, int y) + { + this.X = x; + this.Y = y; + } +} + diff --git a/sandbox/SandboxNet6/Program.cs b/sandbox/SandboxNet6/Program.cs index 05607a61..1ff05c22 100644 --- a/sandbox/SandboxNet6/Program.cs +++ b/sandbox/SandboxNet6/Program.cs @@ -1,7 +1,9 @@ // See https://aka.ms/new-console-template for more information using MemoryPack; using System.Buffers; +using System.Drawing; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; // System.Buffers.IBufferWriter @@ -103,3 +105,75 @@ public partial class GenricUnionB : IGenericUnion } +[MemoryPackable] +public partial struct PartialStructOne +{ + public int X; + public int Y; + + //[MemoryPackConstructor] + public PartialStructOne(int x) + { + this.X = x; + this.Y = 0; + } + + //[MemoryPackConstructor] + public PartialStructOne(int x, int y) + { + this.X = x; + this.Y = y; + } +} + +//[MemoryPackable] +//public partial struct DateTimeParamDefault +//{ +// public DateTimeOffset DateTime; // short offset(2+padding) + dateTime/ulong(8) = 16 +// public long Timestamp; // 8 +// public bool IsItInSeconds; // 1(+padding7) = 8 +//} + +//[StructLayout(LayoutKind.Sequential)] +//[MemoryPackable] +//public partial struct TesTest +//{ +// public ValueTuple VTI; +// public MyMessageHeader MyMsgHead; +// public bool IsItInSeconds; // 1(+padding7) = 8 +//} + +//[StructLayout(LayoutKind.Sequential)] +//[MemoryPackable] +//public partial struct DateTimeParamSequential +//{ +// public DateTimeOffset DateTime; // short offset(2+padding) + dateTime/ulong(8) = 16 +// public long Timestamp; // 8 +// public bool IsItInSeconds; // 1(+padding7) = 8 +//} + +//[StructLayout(LayoutKind.Auto)] +//[MemoryPackable] +//public partial struct DateTimeParamAuto +//{ +// public DateTimeOffset DateTime; // short offset(2+padding) + dateTime/ulong(8) = 16 +// public long Timestamp; // 8 +// public bool IsItInSeconds; // 1(+padding7) = 8 +//} + +//[StructLayout(LayoutKind.Explicit, Size = 25)] +//[MemoryPackable] +//public partial struct DateTimeParamExplicit +//{ +// [FieldOffset(9)] +// public DateTimeOffset DateTime; +// [FieldOffset(1)] +// public long Timestamp; // 8 +// [FieldOffset(0)] +// public bool IsItInSeconds; // 1 +//} + +//[StructLayout(LayoutKind.Auto)] +//public struct MyMessageHeader +//{ +//} diff --git a/src/MemoryPack.Generator/DiagnosticDescriptors.cs b/src/MemoryPack.Generator/DiagnosticDescriptors.cs index ccfc7ac7..fabeeb1f 100644 --- a/src/MemoryPack.Generator/DiagnosticDescriptors.cs +++ b/src/MemoryPack.Generator/DiagnosticDescriptors.cs @@ -276,4 +276,20 @@ internal static class DiagnosticDescriptors category: Category, defaultSeverity: DiagnosticSeverity.Error, isEnabledByDefault: true); + + public static readonly DiagnosticDescriptor UnamangedStructWithLayoutAutoField = new( + id: "MEMPACK034", + title: "Before .NET 7 unmanaged struct must annotate LayoutKind.Auto or Explicit", + messageFormat: "The unmanaged struct '{0}' has LayoutKind.Auto field('{1}'). Before .NET 7, if field contains Auto then automatically promote to LayoutKind.Auto but .NET 7 is Sequential so breaking binary compatibility when runtime upgraded. To safety, you have to annotate [StructLayout(LayoutKind.Auto)] or LayoutKind.Explicit to type.", + category: Category, + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true); + + public static readonly DiagnosticDescriptor UnamangedStructMemoryPackCtor = new( + id: "MEMPACK035", + title: "Unamanged strcut does not allow [MemoryPackConstructor]", + messageFormat: "The unamanged struct '{0}' can not annotate with [MemoryPackConstructor] because don't call any constructors", + category: Category, + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true); } diff --git a/src/MemoryPack.Generator/MemoryPackGenerator.Parser.cs b/src/MemoryPack.Generator/MemoryPackGenerator.Parser.cs index 3350d389..59aef191 100644 --- a/src/MemoryPack.Generator/MemoryPackGenerator.Parser.cs +++ b/src/MemoryPack.Generator/MemoryPackGenerator.Parser.cs @@ -2,6 +2,8 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using System.ComponentModel; +using System.Reflection; +using System.Runtime.InteropServices; using System.Runtime.Serialization; using System.Text; @@ -40,7 +42,7 @@ public enum MemberKind CustomFormatter, // used [MemoryPackCustomFormatterAttribtue] } -partial class TypeMeta +public partial class TypeMeta { DiagnosticDescriptor? ctorInvalid = null; @@ -145,13 +147,22 @@ public TypeMeta(INamedTypeSymbol symbol, ReferenceSymbols reference) return null; // allows null as ok(not exists explicitly declared constructor == has implictly empty ctor) } - if (ctors.Length == 1) + if (!Symbol.IsUnmanagedType && ctors.Length == 1) { return ctors[0]; } var ctorWithAttrs = ctors.Where(x => x.ContainsAttribute(reference.MemoryPackConstructorAttribute)).ToArray(); + if (Symbol.IsUnmanagedType) + { + if (ctorWithAttrs.Length != 0) + { + ctorInvalid = DiagnosticDescriptors.UnamangedStructMemoryPackCtor; + } + return null; + } + if (ctorWithAttrs.Length == 0) { ctorInvalid = DiagnosticDescriptors.MultipleCtorWithoutAttribute; @@ -274,23 +285,86 @@ public bool Validate(TypeDeclarationSyntax syntax, IGeneratorContext context, bo noError = false; } + // ctor if (ctorInvalid != null) { context.ReportDiagnostic(Diagnostic.Create(ctorInvalid, syntax.Identifier.GetLocation(), Symbol.Name)); noError = false; } - // check ctor members - if (this.Constructor != null) + if (this.IsUnmanagedType) + { + var structLayoutFields = this.Symbol.GetAllMembers() + .OfType() + .Select(x => + { + if (x.IsStatic) return null; + + // ValueTuple, DateTime, DateTimeOffset is auto but can not get from Roslyn GetAttributes. + + if (SymbolEqualityComparer.Default.Equals(x.Type, reference.KnownTypes.System_DateTime) || SymbolEqualityComparer.Default.Equals(x.Type, reference.KnownTypes.System_DateTimeOffset)) + { + return Tuple.Create(x.Type, LayoutKind.Auto); + } + + if (x.Type is INamedTypeSymbol nts && nts.IsGenericType) + { + var fullyQualifiedString = nts.ConstructUnboundGenericType().FullyQualifiedToString(); + if (fullyQualifiedString.StartsWith("global::System.ValueTuple<")) + { + return Tuple.Create(x.Type, LayoutKind.Auto); + } + } + + var structLayout = x.Type.GetAttribute(reference.KnownTypes.System_Runtime_InteropServices_StructLayout); + var layoutKind = (structLayout != null && structLayout.ConstructorArguments.Length != 0) + ? structLayout.ConstructorArguments[0].Value + : null; + + if (layoutKind != null) + { + return Tuple.Create(x.Type, (LayoutKind)layoutKind); + } + + return null; + }) + .Where(x => x != null && x.Item2 == LayoutKind.Auto) + .ToArray(); + + // has auto field, should mark Auto in lower Net6 + if (structLayoutFields.Length != 0) + { + var structLayout = Symbol.GetAttribute(reference.KnownTypes.System_Runtime_InteropServices_StructLayout); + var layoutKind = (structLayout != null && structLayout.ConstructorArguments.Length != 0) + ? structLayout.ConstructorArguments[0].Value + : null; + + if (layoutKind == null || (LayoutKind)layoutKind == LayoutKind.Sequential) + { + var autoTypes = string.Join(", ", structLayoutFields.Select(x => x!.Item1.Name)); + + if (!context.IsNet7OrGreater) + { + context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.UnamangedStructWithLayoutAutoField, syntax.Identifier.GetLocation(), Symbol.Name, autoTypes)); + noError = false; + } + } + } + } + else { - var nameDict = new HashSet(Members.Where(x => x.IsConstructorParameter).Select(x => x.Name), StringComparer.OrdinalIgnoreCase); - var allParameterExists = this.Constructor.Parameters.All(x => nameDict.Contains(x.Name)); - if (!allParameterExists) + // check ctor members + if (this.Constructor != null) { - var location = Constructor.Locations.FirstOrDefault() ?? syntax.Identifier.GetLocation(); + var nameDict = new HashSet(Members.Where(x => x.IsConstructorParameter).Select(x => x.Name), StringComparer.OrdinalIgnoreCase); + var allParameterExists = this.Constructor.Parameters.All(x => nameDict.Contains(x.Name)); + if (!allParameterExists) + { + var location = Constructor.Locations.FirstOrDefault() ?? syntax.Identifier.GetLocation(); - context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.ConstructorHasNoMatchedParameter, location, Symbol.Name)); - noError = false; + context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.ConstructorHasNoMatchedParameter, location, Symbol.Name)); + noError = false; + } } } @@ -474,6 +548,11 @@ public bool Validate(TypeDeclarationSyntax syntax, IGeneratorContext context, bo return noError; } + + public override string ToString() + { + return this.TypeName; + } } partial class MemberMeta diff --git a/src/MemoryPack.Generator/ReferenceSymbols.cs b/src/MemoryPack.Generator/ReferenceSymbols.cs index 74753df7..fa0db61c 100644 --- a/src/MemoryPack.Generator/ReferenceSymbols.cs +++ b/src/MemoryPack.Generator/ReferenceSymbols.cs @@ -1,5 +1,6 @@ using Microsoft.CodeAnalysis; using System.Reflection; +using System.Runtime.InteropServices; namespace MemoryPack.Generator; @@ -84,6 +85,10 @@ public class WellKnownTypes public INamedTypeSymbol System_Collections_Generic_KeyValuePair_T { get; } public INamedTypeSymbol System_Nullable_T { get; } + public INamedTypeSymbol System_DateTime { get; } + public INamedTypeSymbol System_DateTimeOffset { get; } + public INamedTypeSymbol System_Runtime_InteropServices_StructLayout { get; } + // netstandard2.0 source generator has there reference so use string instead... //public INamedTypeSymbol System_Memory_T { get; } //public INamedTypeSymbol System_ReadOnlyMemory_T { get; } @@ -199,6 +204,10 @@ public WellKnownTypes(ReferenceSymbols parent) //System_Buffers_ReadOnlySequence_T = GetTypeByMetadataName("System.Buffers.ReadOnlySequence").ConstructUnboundGenericType(); //System_Collections_Generic_PriorityQueue_T = GetTypeByMetadataName("System.Collections.Generic.PriorityQueue").ConstructUnboundGenericType(); + System_DateTime = GetTypeByMetadataName("System.DateTime"); + System_DateTimeOffset = GetTypeByMetadataName("System.DateTimeOffset"); + System_Runtime_InteropServices_StructLayout = GetTypeByMetadataName("System.Runtime.InteropServices.StructLayoutAttribute"); + knownTypes = new HashSet(new[] { System_Collections_Generic_IEnumerable_T, diff --git a/tests/MemoryPack.Tests/GeneratorDiagnosticsTest.TypeScript.cs b/tests/MemoryPack.Tests/GeneratorDiagnosticsTest.TypeScript.cs index fdb3cc49..8e036431 100644 --- a/tests/MemoryPack.Tests/GeneratorDiagnosticsTest.TypeScript.cs +++ b/tests/MemoryPack.Tests/GeneratorDiagnosticsTest.TypeScript.cs @@ -21,7 +21,7 @@ public partial class GeneratorDiagnosticsTest { void Compile2(int id, string code, bool allowMultipleError = false) { - var diagnostics = CSharpGeneratorRunner.RunGenerator(code, new TypeScriptOptionProvider()); + var diagnostics = CSharpGeneratorRunner.RunGenerator(code, options: new TypeScriptOptionProvider()); if (!allowMultipleError) { diagnostics.Length.Should().Be(1); @@ -41,7 +41,7 @@ string CompileAndRead(string code, string fileName, bool enableNullableTypes = t optionProvider["build_property.MemoryPackGenerator_TypeScriptOutputDirectory"] = outputDir; optionProvider["build_property.MemoryPackGenerator_TypeScriptEnableNullableTypes"] = enableNullableTypes ? "true" : "false"; - CSharpGeneratorRunner.RunGenerator(code, optionProvider); + CSharpGeneratorRunner.RunGenerator(code,options: optionProvider); var outputFilePath = Path.Combine(outputDir, fileName); diff --git a/tests/MemoryPack.Tests/GeneratorDiagnosticsTest.cs b/tests/MemoryPack.Tests/GeneratorDiagnosticsTest.cs index 6668c318..03ea29df 100644 --- a/tests/MemoryPack.Tests/GeneratorDiagnosticsTest.cs +++ b/tests/MemoryPack.Tests/GeneratorDiagnosticsTest.cs @@ -526,6 +526,57 @@ public Hoge(int prop1, int prop2) } """); } + + [Fact] + public void MEMPACK034_UnamangedStructWithLayoutAutoField() + { + var code = """ +using System; +using MemoryPack; + +[MemoryPackable] +public partial struct Hoge +{ + public int X; + public int Y; + public DateTime DT; +} +"""; + + { + var diagnostics = CSharpGeneratorRunner.RunGenerator(code, preprocessorSymbols: new[] { "NET7_0_OR_GREATER" }); + diagnostics.Length.Should().Be(0); + } + { + var diagnostics = CSharpGeneratorRunner.RunGenerator(code, preprocessorSymbols: new string[] { }); + diagnostics.Length.Should().Be(1); + diagnostics[0].Id.Should().Be("MEMPACK034"); + } + } + + [Fact] + public void MEMPACK035_UnamangedStructMemoryPackCtor() + { + Compile(35, """ +using MemoryPack; + +[MemoryPackable] +public partial struct Hoge +{ + public int X; + public int Y; + + [MemoryPackConstructor] + public Hoge(int x, int y) + { + this.X = x; + this.Y = y; + } +} +"""); + } + + } diff --git a/tests/MemoryPack.Tests/Utils/CSharpGeneratorRunner.cs b/tests/MemoryPack.Tests/Utils/CSharpGeneratorRunner.cs index 3ebc3d69..b90fecec 100644 --- a/tests/MemoryPack.Tests/Utils/CSharpGeneratorRunner.cs +++ b/tests/MemoryPack.Tests/Utils/CSharpGeneratorRunner.cs @@ -37,15 +37,21 @@ public static void InitializeCompilation() baseCompilation = compilation; } - public static Diagnostic[] RunGenerator(string source, AnalyzerConfigOptionsProvider? options = null) + public static Diagnostic[] RunGenerator(string source, string[]? preprocessorSymbols = null, AnalyzerConfigOptionsProvider? options = null) { - var driver = CSharpGeneratorDriver.Create(new MemoryPackGenerator()); + if (preprocessorSymbols == null) + { + preprocessorSymbols = new[] { "NET7_0_OR_GREATER" }; + } + var parseOptions = new CSharpParseOptions(LanguageVersion.CSharp11, preprocessorSymbols: preprocessorSymbols); + + var driver = CSharpGeneratorDriver.Create(new MemoryPackGenerator()).WithUpdatedParseOptions(parseOptions); if (options != null) { driver = (Microsoft.CodeAnalysis.CSharp.CSharpGeneratorDriver)driver.WithUpdatedAnalyzerConfigOptions(options); } - var compilation = baseCompilation.AddSyntaxTrees(CSharpSyntaxTree.ParseText(source, new CSharpParseOptions(LanguageVersion.CSharp11))); // use C#11 + var compilation = baseCompilation.AddSyntaxTrees(CSharpSyntaxTree.ParseText(source, parseOptions)); driver.RunGeneratorsAndUpdateCompilation(compilation, out var newCompilation, out var diagnostics);