Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 11 additions & 17 deletions src/DotLLM.Models/Gguf/GgufReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ namespace DotLLM.Models.Gguf;

/// <summary>
/// Static binary parser for the GGUF file format. Pure functions: bytes in, structs out.
/// Handles both GGUF v2 (uint32 counts) and v3 (uint64 counts).
/// Supports GGUF v2 and v3, which are identical on the wire: tensor/metadata counts,
/// string lengths and array lengths are all <c>uint64</c>. (The <c>uint32</c> form belongs to
/// the obsolete v1, which the header validation rejects.)
/// </summary>
public static class GgufReader
{
Expand All @@ -30,19 +32,9 @@ public static GgufHeader ReadHeader(BinaryReader reader)
throw new InvalidDataException(
$"Unsupported GGUF version: {version}. Only versions 2 and 3 are supported.");

ulong tensorCount;
ulong metadataKvCount;

if (version == 2)
{
tensorCount = reader.ReadUInt32();
metadataKvCount = reader.ReadUInt32();
}
else
{
tensorCount = reader.ReadUInt64();
metadataKvCount = reader.ReadUInt64();
}
// GGUF v2 and v3 both store these counts as uint64 (the uint32 form was v1 only).
ulong tensorCount = reader.ReadUInt64();
ulong metadataKvCount = reader.ReadUInt64();

return new GgufHeader(version, tensorCount, metadataKvCount);
}
Expand Down Expand Up @@ -109,11 +101,12 @@ public static List<GgufTensorDescriptor> ReadTensorInfos(BinaryReader reader, Gg
}

/// <summary>
/// Reads a GGUF length-prefixed UTF-8 string. V2 uses uint32 length, v3 uses uint64.
/// Reads a GGUF length-prefixed UTF-8 string. The length is uint64 in both supported
/// versions (v2 and v3); only the obsolete v1 used a uint32 length.
/// </summary>
internal static string ReadGgufString(BinaryReader reader, uint version)
{
ulong length = version == 2 ? reader.ReadUInt32() : reader.ReadUInt64();
ulong length = reader.ReadUInt64();

if (length == 0)
return string.Empty;
Expand Down Expand Up @@ -149,7 +142,8 @@ private static object ReadMetadataValue(BinaryReader reader, uint version, GgufV
private static object ReadArray(BinaryReader reader, uint version)
{
var elementType = (GgufValueType)reader.ReadUInt32();
ulong count = version == 2 ? reader.ReadUInt32() : reader.ReadUInt64();
// Array length is uint64 in both supported versions (v2 and v3); v1 used uint32.
ulong count = reader.ReadUInt64();

if (count > int.MaxValue)
throw new InvalidDataException($"GGUF array length {count} exceeds Int32.MaxValue.");
Expand Down
32 changes: 32 additions & 0 deletions tests/DotLLM.Tests.Unit/Models/Gguf/GgufFileTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,38 @@ public void Open_WithTensors_ProvidesDataPointer()
}
}

[Fact]
public void Open_V2File_WithArrayMetadataAndTensors_Succeeds()
{
// Reproduces the real-world GGUF v2 failure (e.g. TheBloke/Llama-2-13B-GGUF): before the
// uint64 width fix, the reader mis-parsed v2 string/array lengths, producing garbage tensor
// offsets that tripped GgufFile.Open's "data extends beyond file boundary" guard (with an
// empty tensor name). A correctly parsed v2 file opens and exposes its tensors.
byte[] embd = new byte[8 * 16 * 4];
for (int i = 0; i < embd.Length; i++) embd[i] = (byte)((i % 255) + 1);

var data = new GgufTestData(version: 2)
.AddString("general.architecture", "llama")
.AddStringArray("tokenizer.ggml.tokens", ["<s>", "</s>", "hello", "world"])
.AddTensor("token_embd.weight", [8, 16], 0, embd) // F32
.AddTensor("output_norm.weight", [16], 0, new byte[16 * 4]); // F32
string path = WriteTempGguf(data);

using var file = GgufFile.Open(path);

Assert.Equal(2u, file.Header.Version);
Assert.Equal(2ul, file.Header.TensorCount);
Assert.Equal("llama", file.Metadata.GetString("general.architecture"));
Assert.True(file.TensorsByName.ContainsKey("token_embd.weight"));
Assert.True(file.TensorsByName.ContainsKey("output_norm.weight"));
Assert.NotEqual(nint.Zero, file.DataBasePointer);
unsafe
{
byte* ptr = (byte*)file.DataBasePointer;
Assert.Equal(1, ptr[0]); // first byte of token_embd data
}
}

[Fact]
public void Open_TensorsByName_LookupWorks()
{
Expand Down
48 changes: 48 additions & 0 deletions tests/DotLLM.Tests.Unit/Models/Gguf/GgufReaderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,54 @@ public void ReadMetadata_V2_ScalarTypes()
Assert.Equal(42u, metadata["count"].Value);
}

/// <summary>
/// Regression for the GGUF v2 width bug: v2 stores counts, string lengths and array lengths
/// as <c>uint64</c> (identical to v3 — the <c>uint32</c> form was v1 only). The reader and the
/// in-memory test writer both previously treated v2 as uint32, so they agreed with each other
/// while disagreeing with the spec — and every real v2 file (e.g. TheBloke's Llama-2-13B-GGUF)
/// mis-parsed: a uint32 string length consumed 4 bytes where the file wrote 8, cascading through
/// the string-array metadata into garbage tensor names and offsets (the latter surfaced as a
/// bogus "data extends beyond file boundary" with an empty tensor name). A scalar-only v2 case
/// can't catch this — it needs a multi-element array followed by tensor infos so misalignment
/// compounds. This asserts a full v2 parse (header → array metadata → tensor infos) is exact.
/// </summary>
[Fact]
public void Reads_V2_FullFile_ArrayMetadataThenTensors_Exactly()
{
var data = new GgufTestData(version: 2)
.AddString("general.architecture", "llama")
.AddStringArray("tokenizer.ggml.tokens", ["<s>", "</s>", "hello", "world"])
.AddUInt32("llama.block_count", 2)
.AddTensor("token_embd.weight", [8, 16], 0, new byte[8 * 16 * 4]) // F32
.AddTensor("blk.0.attn_q.weight", [16, 16], 1, new byte[16 * 16 * 2]) // F16
.AddTensor("output_norm.weight", [16], 0, new byte[16 * 4]); // F32
byte[] bytes = data.Build();

using var stream = new MemoryStream(bytes);
using var reader = new BinaryReader(stream);

var header = GgufReader.ReadHeader(reader);
Assert.Equal(2u, header.Version);
Assert.Equal(3ul, header.TensorCount);
Assert.Equal(3ul, header.MetadataKvCount);

var metadata = GgufReader.ReadMetadata(reader, header);
Assert.Equal("llama", metadata["general.architecture"].Value);
Assert.Equal(["<s>", "</s>", "hello", "world"],
(string[])metadata["tokenizer.ggml.tokens"].Value);
Assert.Equal(2u, metadata["llama.block_count"].Value);

var tensors = GgufReader.ReadTensorInfos(reader, header);
Assert.Equal(3, tensors.Count);
Assert.Equal("token_embd.weight", tensors[0].Name);
Assert.Equal("blk.0.attn_q.weight", tensors[1].Name);
Assert.Equal("output_norm.weight", tensors[2].Name);
// Offsets are sequential blobs (no alignment between them in the writer).
Assert.Equal(0ul, tensors[0].DataOffset);
Assert.Equal((ulong)(8 * 16 * 4), tensors[1].DataOffset);
Assert.Equal((ulong)(8 * 16 * 4 + 16 * 16 * 2), tensors[2].DataOffset);
}

#endregion

#region Tensor Infos
Expand Down
21 changes: 5 additions & 16 deletions tests/DotLLM.Tests.Unit/Models/Gguf/GgufTestData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,20 +144,11 @@ public byte[] Build()
_stream.SetLength(0);
_stream.Position = 0;

// Header
// Header. GGUF v2 and v3 both store counts as uint64 (only the obsolete v1 used uint32).
_writer.Write(GgufReader.GgufMagic);
_writer.Write(_version);

if (_version == 2)
{
_writer.Write((uint)_tensorWriters.Count);
_writer.Write((uint)_metadataWriters.Count);
}
else
{
_writer.Write((ulong)_tensorWriters.Count);
_writer.Write((ulong)_metadataWriters.Count);
}
_writer.Write((ulong)_tensorWriters.Count);
_writer.Write((ulong)_metadataWriters.Count);

// Metadata
foreach (var writeMetadata in _metadataWriters)
Expand Down Expand Up @@ -200,10 +191,8 @@ private void WriteGgufString(BinaryWriter writer, string value)

private void WriteLength(BinaryWriter writer, ulong length)
{
if (_version == 2)
writer.Write((uint)length);
else
writer.Write(length);
// String and array lengths are uint64 in both supported versions (v2 and v3); v1 used uint32.
writer.Write(length);
}

private static long AlignUp(long value, uint alignment)
Expand Down