diff --git a/LibLouis.NET.Test/LiblouisHeapTests.cs b/LibLouis.NET.Test/LiblouisHeapTests.cs new file mode 100644 index 0000000..6bbfe70 --- /dev/null +++ b/LibLouis.NET.Test/LiblouisHeapTests.cs @@ -0,0 +1,153 @@ +using System; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices; +using System.Text; + +using Xunit; + +namespace LibLouis.NET.Test; + +/// +/// lou_findTable's result is the caller's to free, with liblouis's own C runtime. On Windows that +/// is found through liblouis's import table, so the table walk is tested here against a synthetic +/// image on every platform, and against the real liblouis.dll on Windows. +/// +public unsafe class LiblouisHeapTests +{ + private static readonly string[] Tables = ["da-dk-g26.ctb", "da-dk-g16-markers.ctb"]; + + private static string[] TablePaths() => + [.. Tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "tables", t))]; + + // RVAs in the synthetic image. + private const int ImportTable = 0x200; + private const int NameThunks = 0x300; + private const int AddressThunks = 0x400; + private const int MallocName = 0x500; + private const int FreeName = 0x520; + private const int LibraryName = 0x540; + + /// + /// A minimal mapped PE image importing, from one library: an ordinal, malloc and free, bound to + /// 0x1111, 0x2222 and 0x3333. + /// + private static byte[] SyntheticImage() + { + byte[] image = new byte[0x600]; + Span span = image; + + BitConverter.TryWriteBytes(span[0x00..], (ushort)0x5A4D); // "MZ" + BitConverter.TryWriteBytes(span[0x3C..], 0x40); // e_lfanew + BitConverter.TryWriteBytes(span[0x40..], 0x00004550u); // "PE\0\0" + + // Optional header at 0x40 + 24; the import directory is data directory 1. + int dataDirectories = 0x40 + 24 + (IntPtr.Size == 8 ? 112 : 96); + BitConverter.TryWriteBytes(span[(dataDirectories + 8)..], ImportTable); + + // One descriptor, then the all-zero terminator. + BitConverter.TryWriteBytes(span[(ImportTable + 0)..], NameThunks); + BitConverter.TryWriteBytes(span[(ImportTable + 12)..], LibraryName); + BitConverter.TryWriteBytes(span[(ImportTable + 16)..], AddressThunks); + + nint ordinal = IntPtr.Size == 8 ? unchecked((nint)0x8000_0000_0000_0007) : unchecked((nint)0x8000_0007); + WriteThunks(span[NameThunks..], [ordinal, MallocName, FreeName, 0]); + WriteThunks(span[AddressThunks..], [0x1111, 0x2222, 0x3333, 0]); + + // IMAGE_IMPORT_BY_NAME: a two byte hint, then the name. + Encoding.ASCII.GetBytes("malloc\0").CopyTo(span[(MallocName + 2)..]); + Encoding.ASCII.GetBytes("free\0").CopyTo(span[(FreeName + 2)..]); + Encoding.ASCII.GetBytes("msvcrt.dll\0").CopyTo(span[LibraryName..]); + + return image; + } + + private static void WriteThunks(Span destination, nint[] thunks) + { + MemoryMarshal.AsBytes(thunks.AsSpan()).CopyTo(destination); + } + + [Theory] + [InlineData("free", 0x3333)] + [InlineData("malloc", 0x2222)] + [InlineData("realloc", 0)] + [InlineData("fre", 0)] + public void FindImport_ReturnsTheBoundAddressOfANamedImport(string name, int expected) + { + fixed (byte* image = SyntheticImage()) + { + Assert.Equal(expected, (int)LiblouisHeap.FindImport(image, Encoding.ASCII.GetBytes(name))); + } + } + + /// + /// Without OriginalFirstThunk the names have been overwritten by addresses, so the descriptor + /// cannot be searched - it must be skipped rather than read as names. + /// + [Fact] + public void FindImport_SkipsADescriptorWithoutNames() + { + byte[] bytes = SyntheticImage(); + BitConverter.TryWriteBytes(bytes.AsSpan(ImportTable), 0); + + fixed (byte* image = bytes) + { + Assert.Equal(0, (int)LiblouisHeap.FindImport(image, "free"u8)); + } + } + + [Fact] + public void FindImport_RejectsSomethingThatIsNotAPeImage() + { + fixed (byte* image = new byte[0x600]) + { + Assert.Equal(0, (int)LiblouisHeap.FindImport(image, "free"u8)); + } + } + + /// + /// The real liblouis.dll has to import free() from a C runtime, or every lou_findTable result + /// would be left unfreed. + /// + [Fact] + public void FindImport_FindsFreeInTheLiblouisWeShip() + { + if (!OperatingSystem.IsWindows()) + { + return; + } + + // Make sure the P/Invoke layer has loaded it. + _ = LibLouis.Instance.Version; + + nint module = NativeLibrary.Load("liblouis", typeof(LibLouis).Assembly, DllImportSearchPath.SafeDirectories); + nint free = LiblouisHeap.FindImport((byte*)module, "free"u8); + + nint[] runtimeFrees = [.. new[] { "msvcrt.dll", "ucrtbase.dll" } + .Select(name => NativeLibrary.TryLoad(name, out nint runtime) ? NativeLibrary.GetExport(runtime, "free") : 0)]; + + Assert.NotEqual(0, free); + Assert.Contains(free, runtimeFrees); + } + + /// + /// Each result is freed into liblouis's heap as it comes back. Releasing it into a different + /// heap corrupts the one liblouis translates with, so interleave the two and repeat enough for + /// the corruption to surface as a crash. + /// + [Fact] + public void FindTable_ResultsCanBeFreedBetweenTranslations() + { + LibLouis.Instance.IndexTables(TablePaths()); + + for (int k = 0; k < 2000; k++) + { + string? table = LibLouis.Instance.FindTable("language:da type:literary grade:2"); + + Assert.NotNull(table); + Assert.EndsWith("da-dk-g26.ctb", table, StringComparison.Ordinal); + + Assert.NotEmpty(LibLouis.Instance.Translate(TablePaths(), "Første linje", 64, null, null, TranslationMode.Regular)); + } + } +} diff --git a/LibLouis.NET.Test/NativeLockTests.cs b/LibLouis.NET.Test/NativeLockTests.cs index d36966c..b1765b1 100644 --- a/LibLouis.NET.Test/NativeLockTests.cs +++ b/LibLouis.NET.Test/NativeLockTests.cs @@ -180,7 +180,7 @@ private static string ReadLibrarySource(string fileName) { DirectoryInfo? directory = new(AppContext.BaseDirectory); - while (directory is not null && !File.Exists(Path.Combine(directory.FullName, "LibLouis.NET.sln"))) + while (directory is not null && !File.Exists(Path.Combine(directory.FullName, "LibLouis.NET.slnx"))) { directory = directory.Parent; } diff --git a/LibLouis.NET.Test/PositionMappingTests.cs b/LibLouis.NET.Test/PositionMappingTests.cs index aae28b8..1cc6f70 100644 --- a/LibLouis.NET.Test/PositionMappingTests.cs +++ b/LibLouis.NET.Test/PositionMappingTests.cs @@ -178,4 +178,77 @@ public void CursorPositionIsAnIndexIntoTheOutput() Assert.InRange(result.CursorPosition, 0, result.Output.Length - 1); Assert.False(char.IsLowSurrogate(result.Output[result.CursorPosition])); } + + /// + /// A cursor after the last character - where it sits while appending - has to stay after the + /// last character, not move back onto it. + /// + [Theory] + [InlineData("abc")] + [InlineData("a\U0001D11E")] + [InlineData("")] + public void CursorAtEndOfInputStaysAtEndOfOutput(string input) + { + int outputLength = Math.Max(16, input.Length * 4); + + TranslatedString result = LibLouis.Instance.Translate( + TablePaths(), + input, + outputLength, + null, + null, + new int[input.Length], + new int[outputLength], + input.Length, + TranslationMode.Regular); + + Assert.Equal(result.Output.Length, result.CursorPosition); + } + + /// + /// The same going the other way. + /// + [Fact] + public void BackTranslateCursorAtEndOfInputStaysAtEndOfOutput() + { + string braille = Translate("abc").Output; + int outputLength = braille.Length * 4; + + TranslatedString result = LibLouis.Instance.BackTranslate( + TablePaths(), + braille, + outputLength, + null, + null, + new int[braille.Length], + new int[outputLength], + braille.Length, + TranslationMode.Regular); + + Assert.Equal(result.Output.Length, result.CursorPosition); + } + + /// + /// The last character is still a character: only the position after it is the end. + /// + [Fact] + public void CursorOnLastCharacterStaysOnIt() + { + const string input = "abc"; + + int outputLength = input.Length * 4; + + TranslatedString result = LibLouis.Instance.Translate( + TablePaths(), + input, + outputLength, + null, + null, + new int[input.Length], + new int[outputLength], + input.Length - 1, + TranslationMode.Regular); + + Assert.Equal(result.OutputPosition[^1], result.CursorPosition); + } } diff --git a/LibLouis.NET.Test/ReturnedStringOwnershipTests.cs b/LibLouis.NET.Test/ReturnedStringOwnershipTests.cs index 4c3a811..16fd534 100644 --- a/LibLouis.NET.Test/ReturnedStringOwnershipTests.cs +++ b/LibLouis.NET.Test/ReturnedStringOwnershipTests.cs @@ -6,15 +6,13 @@ namespace LibLouis.NET.Test; /// -/// liblouis owns every string it returns, so the wrapper must not hand those pointers to the -/// marshaller's Free. +/// The strings liblouis returns must not be freed by the default marshaller. /// /// * lou_setDataPath / lou_getDataPath return a pointer into a static char[MAXSTRING] inside /// liblouis (compileTranslationTable.c:59-73). Passing that to free() is undefined behaviour /// on every platform. -/// * lou_findTable returns malloc'd memory. Our Windows binaries are built with mingw-w64 and -/// allocate from msvcrt.dll, while .NET frees through ucrtbase.dll - different heaps, so -/// freeing it from managed code corrupts the heap there. +/// * lou_findTable returns malloc'd memory the caller frees, but with liblouis's C runtime +/// rather than the CoTaskMemFree .NET would use on Windows - see LiblouisHeapTests. /// public class ReturnedStringOwnershipTests { diff --git a/LibLouis.NET/LibLouis.cs b/LibLouis.NET/LibLouis.cs index 6d8e661..82dc013 100644 --- a/LibLouis.NET/LibLouis.cs +++ b/LibLouis.NET/LibLouis.cs @@ -367,7 +367,8 @@ public TranslatedString Translate( byte[] outputBuffer = PrepareUCSOutputBuffer(outputBufferLength); TypeForm[]? typeFormBuffer = PrepareTypeFormBuffer(formtype, inputLength, outputBufferLength); - // The cursor arrives as a .NET string index and liblouis wants a widechar index. + // The cursor arrives as a .NET string index and liblouis wants a widechar index. A cursor + // at the end of the input is not passed on, but placed at the end of the output below. int[] inputOffsets = Utf16OffsetOfWidechar(input); int widecharCursor = ToWidecharCursor(input, cursorPosition); @@ -393,7 +394,7 @@ public TranslatedString Translate( return new TranslatedString { Output = output, - CursorPosition = mappedCursor, + CursorPosition = IsCursorAtEnd(input, cursorPosition) ? output.Length : mappedCursor, InputPosition = mappedInputPosition, OutputPosition = mappedOutputPosition, OutputDots78 = ExtractOutputDots78(typeFormBuffer, outputLength), @@ -512,7 +513,8 @@ public TranslatedString BackTranslate( byte[] outputBuffer = PrepareUCSOutputBuffer(outputBufferLength); TypeForm[]? typeFormBuffer = PrepareTypeFormBuffer(formtype, inputLength, outputBufferLength); - // The cursor arrives as a .NET string index and liblouis wants a widechar index. + // The cursor arrives as a .NET string index and liblouis wants a widechar index. A cursor + // at the end of the input is not passed on, but placed at the end of the output below. int[] inputOffsets = Utf16OffsetOfWidechar(input); int widecharCursor = ToWidecharCursor(input, cursorPosition); @@ -538,7 +540,7 @@ public TranslatedString BackTranslate( return new TranslatedString { Output = output, - CursorPosition = mappedCursor, + CursorPosition = IsCursorAtEnd(input, cursorPosition) ? output.Length : mappedCursor, InputPosition = mappedInputPosition, OutputPosition = mappedOutputPosition, }; @@ -708,22 +710,31 @@ public string Hyphenate(IEnumerable tableList, string input, Translation return dots; } + /// + /// Whether the cursor sits at the end of - after its last character, + /// where a caller appending text puts it. + /// + private static bool IsCursorAtEnd(string input, int cursorPosition) => cursorPosition >= input.Length; + /// /// Converts a cursor given as a .NET string index into the widechar index liblouis expects. /// /// - /// Negative means "no cursor" to liblouis and is passed through untouched. + /// Returns -1, "no cursor" to liblouis, for a negative cursor and for one at the end of the + /// input. liblouis reports the cursor back by reading outputPos[*cursorPos] + /// (lou_translateString.c:1389, lou_backTranslateString.c:350), which is only in bounds for a + /// character of the input: at the end it reads one past the array, and for anything negative + /// but -1 it reads before it. The caller places an end cursor itself, see + /// . /// private int ToWidecharCursor(string input, int cursorPosition) { - if (cursorPosition < 0 || input.Length == 0) + if (cursorPosition < 0 || IsCursorAtEnd(input, cursorPosition)) { - return cursorPosition; + return -1; } - int[] widechars = WidecharOfUtf16Offset(input); - - return widechars[Math.Clamp(cursorPosition, 0, input.Length - 1)]; + return WidecharOfUtf16Offset(input)[cursorPosition]; } /// @@ -779,7 +790,7 @@ private int ToWidecharCursor(string input, int cursorPosition) inputPosition[t] = inputOffsets[Math.Clamp(character, 0, lastInputWidechar)]; } - // A negative cursor means "no cursor" to liblouis; leave it alone. + // -1 means "no cursor" to liblouis; leave it alone. int cursorPosition = widecharCursor < 0 || output.Length == 0 ? widecharCursor : outputOffsets[Math.Clamp(widecharCursor, 0, lastOutputWidechar)]; diff --git a/LibLouis.NET/LiblouisHeap.cs b/LibLouis.NET/LiblouisHeap.cs new file mode 100644 index 0000000..84316b6 --- /dev/null +++ b/LibLouis.NET/LiblouisHeap.cs @@ -0,0 +1,142 @@ +using System; +using System.Runtime.InteropServices; + +namespace LibLouis.NET; + +/// +/// Releases memory that liblouis allocated and hands to the caller to free, through the C runtime +/// liblouis allocated it from. +/// +/// +/// liblouis has no function of its own for this: lou_findTable and friends return memory from +/// their C runtime's malloc and expect the caller to call that runtime's free. +/// +/// Off Windows there is one C runtime per process, and is its +/// free(). +/// +/// On Windows there can be several, each with its own heap, and which one liblouis uses depends on +/// how it was built: our x64 and x86 binaries come from mingw-w64 and use msvcrt.dll, win-arm64 +/// comes from llvm-mingw and uses the UCRT. .NET's own string marshalling frees with +/// CoTaskMemFree, which is neither. Releasing memory into the wrong heap corrupts it, so rather +/// than guess the runtime from the RID, this reads liblouis's import table and calls the exact +/// free() its own calls resolve to. +/// +internal static unsafe class LiblouisHeap +{ + private const ushort DosSignature = 0x5A4D; // "MZ" + private const uint PeSignature = 0x00004550; // "PE\0\0" + private const int ImportDirectoryIndex = 1; + + private static readonly Lazy WindowsFree = new(FindImportedFree); + + /// + /// Frees , which liblouis allocated. A null pointer is ignored. + /// + /// + /// On a Windows liblouis that does not import free() - one linked against a static C runtime - + /// there is no way to reach its heap, and the memory is left alone rather than released into + /// the wrong one. None of our builds are linked that way. + /// + public static void Free(void* pointer) + { + if (pointer is null) + { + return; + } + + if (!OperatingSystem.IsWindows()) + { + NativeMemory.Free(pointer); + return; + } + + nint free = WindowsFree.Value; + + if (free != 0) + { + ((delegate* unmanaged[Cdecl])free)(pointer); + } + } + + /// + /// The address liblouis's import of free() is bound to, or 0 if it imports none. + /// + private static nint FindImportedFree() + { + // The same search the P/Invoke stubs use, so this is the module they already loaded. + nint module = NativeLibrary.Load("liblouis", typeof(LiblouisHeap).Assembly, DllImportSearchPath.SafeDirectories); + + return FindImport((byte*)module, "free"u8); + } + + /// + /// Looks up in the import table of the PE image mapped at + /// , and returns what the loader bound it to. + /// + /// + /// Walks the image as loaded, so every offset is an RVA from . The + /// image matches the process's bitness - it could not have been loaded otherwise - so thunks + /// are pointer sized and an import by ordinal is one with the top bit set. + /// + internal static nint FindImport(byte* image, ReadOnlySpan name) + { + if (*(ushort*)image != DosSignature) + { + return 0; + } + + byte* ntHeaders = image + *(int*)(image + 0x3C); + + if (*(uint*)ntHeaders != PeSignature) + { + return 0; + } + + // PE signature (4) + file header (20), then the optional header, whose data directories + // sit after 96 bytes in PE32 and 112 in PE32+. + byte* optionalHeader = ntHeaders + 24; + int dataDirectories = sizeof(nint) == 8 ? 112 : 96; + uint importTable = *(uint*)(optionalHeader + dataDirectories + (ImportDirectoryIndex * 8)); + + if (importTable == 0) + { + return 0; + } + + // IMAGE_IMPORT_DESCRIPTOR: OriginalFirstThunk, TimeDateStamp, ForwarderChain, Name, + // FirstThunk - five uints, ended by an all-zero entry. + for (uint* descriptor = (uint*)(image + importTable); descriptor[3] != 0; descriptor += 5) + { + // OriginalFirstThunk keeps the names; FirstThunk is the table the loader overwrote + // with addresses. Without the former the names are gone. + uint names = descriptor[0]; + uint addresses = descriptor[4]; + + if (names == 0) + { + continue; + } + + nint* nameThunks = (nint*)(image + names); + nint* addressThunks = (nint*)(image + addresses); + + for (int k = 0; nameThunks[k] != 0; k++) + { + if (nameThunks[k] < 0) + { + continue; // imported by ordinal + } + + // IMAGE_IMPORT_BY_NAME: a two byte hint, then the NUL terminated name. + byte* importName = image + (uint)nameThunks[k] + 2; + + if (MemoryMarshal.CreateReadOnlySpanFromNullTerminated(importName).SequenceEqual(name)) + { + return addressThunks[k]; + } + } + } + + return 0; + } +} diff --git a/LibLouis.NET/NativeMethod.cs b/LibLouis.NET/NativeMethod.cs index 95f55df..1445c09 100644 --- a/LibLouis.NET/NativeMethod.cs +++ b/LibLouis.NET/NativeMethod.cs @@ -219,13 +219,12 @@ internal static partial int lou_backTranslateString( internal static partial void lou_indexTables(string?[] tables); /// - /// The best matching table name, or when there is no match. liblouis - /// documents this as the caller's to free, but the memory comes from liblouis's own C runtime - /// - see for why we leak it instead. + /// The best matching table name, or when there is no match. The caller + /// frees it, with liblouis's own C runtime - see . /// [DefaultDllImportSearchPaths(DllImportSearchPath.SafeDirectories)] [LibraryImport("liblouis", EntryPoint = "lou_findTable", StringMarshalling = StringMarshalling.Utf8)] - [return: MarshalUsing(typeof(UTF8StringNoFreeMarshaller))] + [return: MarshalUsing(typeof(UTF8StringLiblouisAllocatedMarshaller))] internal static partial string? lou_findTable(string query); [DefaultDllImportSearchPaths(DllImportSearchPath.SafeDirectories)] diff --git a/LibLouis.NET/Properties/AssemblyInfo.cs b/LibLouis.NET/Properties/AssemblyInfo.cs index 37c5d51..168681a 100644 --- a/LibLouis.NET/Properties/AssemblyInfo.cs +++ b/LibLouis.NET/Properties/AssemblyInfo.cs @@ -1,3 +1,4 @@ using System.Runtime.CompilerServices; [assembly: InternalsVisibleTo("Nota.LibLouis.NET.Test")] +[assembly: InternalsVisibleTo("LibLouis.NET.Test")] diff --git a/LibLouis.NET/TranslatedString.cs b/LibLouis.NET/TranslatedString.cs index 4e82eab..6d0dd6d 100644 --- a/LibLouis.NET/TranslatedString.cs +++ b/LibLouis.NET/TranslatedString.cs @@ -30,6 +30,10 @@ public class TranslatedString /// Where the cursor ended up, as an index into . Negative when the /// translation was given no cursor. /// + /// + /// A cursor at the end of the input - its length - comes back at the end of the output, its + /// length, so text appended there stays after the translation. + /// public required int CursorPosition { get; set; } /// diff --git a/LibLouis.NET/UTF8StringLiblouisAllocatedMarshaller.cs b/LibLouis.NET/UTF8StringLiblouisAllocatedMarshaller.cs new file mode 100644 index 0000000..88c7bbd --- /dev/null +++ b/LibLouis.NET/UTF8StringLiblouisAllocatedMarshaller.cs @@ -0,0 +1,37 @@ +using System.Runtime.InteropServices; +using System.Runtime.InteropServices.Marshalling; + +namespace LibLouis.NET; + +/// +/// Marshals a UTF-8 string that liblouis allocated and the caller has to free, freeing it with +/// liblouis's own C runtime. +/// +/// +/// For lou_findTable and the other functions documented as caller-frees. The default UTF-8 +/// marshalling would free the result too, but with CoTaskMemFree on Windows: a different heap from +/// the one liblouis allocated from, which corrupts it. frees it into the +/// right one. +/// +/// Restricted to for the same reason as +/// : only memory liblouis allocated belongs in its heap. +/// +[CustomMarshaller(typeof(string), MarshalMode.ManagedToUnmanagedOut, typeof(UTF8StringLiblouisAllocatedMarshaller))] +internal static unsafe class UTF8StringLiblouisAllocatedMarshaller +{ + /// + /// Copies the NUL terminated UTF-8 string at into a managed string. + /// + public static string? ConvertToManaged(byte* unmanaged) + { + return Marshal.PtrToStringUTF8((nint)unmanaged); + } + + /// + /// Returns the string to the heap liblouis allocated it from. + /// + public static void Free(byte* unmanaged) + { + LiblouisHeap.Free(unmanaged); + } +} diff --git a/LibLouis.NET/UTF8StringNoFreeMarshaller.cs b/LibLouis.NET/UTF8StringNoFreeMarshaller.cs index 0b5e470..71fbbd8 100644 --- a/LibLouis.NET/UTF8StringNoFreeMarshaller.cs +++ b/LibLouis.NET/UTF8StringNoFreeMarshaller.cs @@ -17,11 +17,8 @@ namespace LibLouis.NET; /// applying it to an input parameter would leak the buffer allocated for every call, so /// parameters keep using the built-in . /// -/// liblouis also has functions whose result the caller *is* expected to free (lou_findTable, -/// lou_findTables, lou_getTableInfo, lou_listTables). Those use this marshaller too: the Windows -/// binaries are built with mingw-w64 and allocate from msvcrt.dll while .NET frees through -/// ucrtbase.dll, so releasing that memory from managed code would corrupt the heap. Leaking a -/// bounded number of small strings is the safer trade. +/// Results the caller *is* expected to free, such as lou_findTable's, use +/// instead. /// [CustomMarshaller(typeof(string), MarshalMode.ManagedToUnmanagedOut, typeof(UTF8StringNoFreeMarshaller))] public static unsafe class UTF8StringNoFreeMarshaller