UB in blob packing (misaligned u32 writes) + signed shift UB in bitset ops

The problem with the misaligned addresses is that OpenType records are tightly packed.

For instance, I'm not sure how to make UBSan happy in this case:

#pragma pack(push, 1)
typedef struct kbts__os2
{
  kbts_u16 Version;
  kbts_s16 AverageCharacterWidth;
  kbts_u16 WeightClass;
  kbts_u16 WidthClass;
  kbts_u16 Type;
  kbts_s16 SubscriptXSize;
  kbts_s16 SubscriptYSize;
  kbts_s16 SubscriptXOffset;
  kbts_s16 SubscriptYOffset;
  kbts_s16 SuperscriptXSize;
  kbts_s16 SuperscriptYSize;
  kbts_s16 SuperscriptXOffset;
  kbts_s16 SuperscriptYOffset;
  kbts_s16 StrikeoutSize;
  kbts_s16 StrikeoutPosition;
  kbts_s16 FamilyClass;
  kbts_u8 Panose[10];
  kbts_u32 UnicodeRange[4]; // <- This is unaligned!
  kbts_u32 VendorId; // <- This is unaligned!
  kbts_u16 Selection;
  kbts_u16 FirstCharacterIndex;
  kbts_u16 LastCharacterIndex;

  // Some version 0 fonts support this, others do not.
  kbts_s16 TypoAscender;
  kbts_s16 TypoDescender;
  kbts_s16 TypoLineGap;
  kbts_u16 WinAscent;
  kbts_u16 WinDescent;

  // If Version >= 1:
  kbts_u32 CodePageRange[2]; // <- This is unaligned!

  // If Version >= 2:
  kbts_s16 Height;
  kbts_s16 CapHeight;
  kbts_u16 DefaultChar;
  kbts_u16 BreakChar;
  kbts_u16 MaxContext;

  // If Version >= 5:
  kbts_u16 LowerOpticalPointSize;
  kbts_u16 UpperOpticalPointSize;
} kbts__os2;

Now, granted, I don't use these values, so a simple solution would be not to touch them, but there's nothing stopping any other u32 anywhere else in the file to be misaligned. Surely, there's a way to tell UBsan that this is explicitly packed and it's fine?

The crash is writing to LookupSubtableIndexOffsets in the output blob, not reading packed font data. Since we control this layout, aligning before the cast fixes it:

OutData = KBTS__ALIGN_POINTER(char, OutData, KBTS_ALIGNOF(kbts_u32));
kbts_u32 *GlyphLookupMatrix = (kbts_u32 *)OutData;

For kbts__os2 that messes with the byteswap reads. Using memcpy internally fixes it, optimized to direct access:

static void kbts__ByteSwapArray16Unchecked(void *Array_, kbts_un Count)
{
    kbts_u8 *Bytes = (kbts_u8 *)Array_;
    KBTS__FOR(It, 0, Count) {
        kbts_u16 V;
        KBTS_MEMCPY(&V, Bytes + It*2, 2);
        V = kbts__ByteSwap16(V);
        KBTS_MEMCPY(Bytes + It*2, &V, 2);
    }
}

For the signed shift, 1 << 31 is UB, just 1u << 31. I forked and put up my modifications for more reference, fixed all the traps I've ran into so far: https://github.com/Pesky01/kb

The crash is writing to LookupSubtableIndexOffsets in the output blob, not reading packed font data. Since we control this layout, aligning before the cast fixes it:

OutData = KBTS__ALIGN_POINTER(char, OutData, KBTS_ALIGNOF(kbts_u32));
kbts_u32 *GlyphLookupMatrix = (kbts_u32 *)OutData;

For kbts__os2 that messes with the byteswap reads. Using memcpy internally fixes it, optimized to direct access:

static void kbts__ByteSwapArray16Unchecked(void *Array_, kbts_un Count)
{
    kbts_u8 *Bytes = (kbts_u8 *)Array_;
    KBTS__FOR(It, 0, Count) {
        kbts_u16 V;
        KBTS_MEMCPY(&V, Bytes + It*2, 2);
        V = kbts__ByteSwap16(V);
        KBTS_MEMCPY(Bytes + It*2, &V, 2);
    }
}

For the signed shift, 1 << 31 is UB, just 1u << 31. I forked and put up my modifications for more reference, fixed all the traps I've ran into so far: https://github.com/Pesky01/kb

Hi, I'm integrating kb_text_shape into dvui (zig immediate mode gui - https://github.com/david-vanderson/dvui/pull/666). Thanks so much for the project!

I also ran into these UB issues and am following along here. @Pesky01's changes work for me, but I'm only exercising a small part of them. Let me know if there's anything I can help with.

Hi, I'm integrating kb_text_shape into dvui (zig immediate mode gui - https://github.com/david-vanderson/dvui/pull/666). Thanks so much for the project!

I also ran into these UB issues and am following along here. @Pesky01's changes work for me, but I'm only exercising a small part of them. Let me know if there's anything I can help with.

The crash is writing to LookupSubtableIndexOffsets in the output blob, not reading packed font data. Since we control this layout, aligning before the cast fixes it:

OutData = KBTS__ALIGN_POINTER(char, OutData, KBTS_ALIGNOF(kbts_u32)); kbts_u32 *GlyphLookupMatrix = (kbts_u32 *)OutData;

For kbts__os2 that messes with the byteswap reads. Using memcpy internally fixes it, optimized to direct access:

static void kbts__ByteSwapArray16Unchecked(void *Array_, kbts_un Count) { kbts_u8 Bytes = (kbts_u8 )Array_; KBTS__FOR(It, 0, Count) { kbts_u16 V; KBTS_MEMCPY(&V, Bytes + It2, 2); V = kbts__ByteSwap16(V); KBTS_MEMCPY(Bytes + It2, &V, 2); } }

For the signed shift, 1 << 31 is UB, just 1u << 31. I forked and put up my modifications for more reference, fixed all the traps I've ran into so far: https://github.com/Pesky01/kb

This is a good starting point, but I'm afraid it doesn't address the more fundamental issue I pointed out above.

I have fonts that put u32s, and arrays of u32s, at non-multiple-of-4 offsets within the file/their respective tables, and, as far as I know, there is nothing stopping any of the offsets in a font file from being the wrong alignment for the data it points to.

If we want to fully avoid this kind of UB (do you have any target platforms for which this is an issue?), I think we either need our own font representation or we need to use ReadXXXUnaligned whenever we access anything in the font.

With that said, in practice, maybe this is only an issue with u32s, and maybe there are few enough of them that always accessing them explicitly unaligned is not too bad of a patch.

The crash is writing to LookupSubtableIndexOffsets in the output blob, not reading packed font data. Since we control this layout, aligning before the cast fixes it:

OutData = KBTS__ALIGN_POINTER(char, OutData, KBTS_ALIGNOF(kbts_u32)); kbts_u32 *GlyphLookupMatrix = (kbts_u32 *)OutData;

For kbts__os2 that messes with the byteswap reads. Using memcpy internally fixes it, optimized to direct access:

static void kbts__ByteSwapArray16Unchecked(void *Array_, kbts_un Count) { kbts_u8 Bytes = (kbts_u8 )Array_; KBTS__FOR(It, 0, Count) { kbts_u16 V; KBTS_MEMCPY(&V, Bytes + It2, 2); V = kbts__ByteSwap16(V); KBTS_MEMCPY(Bytes + It2, &V, 2); } }

For the signed shift, 1 << 31 is UB, just 1u << 31. I forked and put up my modifications for more reference, fixed all the traps I've ran into so far: https://github.com/Pesky01/kb

This is a good starting point, but I'm afraid it doesn't address the more fundamental issue I pointed out above.

I have fonts that put u32s, and arrays of u32s, at non-multiple-of-4 offsets within the file/their respective tables, and, as far as I know, there is nothing stopping any of the offsets in a font file from being the wrong alignment for the data it points to.

If we want to fully avoid this kind of UB (do you have any target platforms for which this is an issue?), I think we either need our own font representation or we need to use ReadXXXUnaligned whenever we access anything in the font.

With that said, in practice, maybe this is only an issue with u32s, and maybe there are few enough of them that always accessing them explicitly unaligned is not too bad of a patch.

This is not exhaustive, but hopefully https://github.com/JimmyLefevre/kb/commit/9e2b6a2c8def3ec56956a6277a81a20a2877a45b covers most cases.

This is not exhaustive, but hopefully https://github.com/JimmyLefevre/kb/commit/9e2b6a2c8def3ec56956a6277a81a20a2877a45b covers most cases.

The crash is writing to LookupSubtableIndexOffsets in the output blob, not reading packed font data. Since we control this layout, aligning before the cast fixes it: OutData = KBTS__ALIGN_POINTER(char, OutData, KBTS_ALIGNOF(kbts_u32)); kbts_u32 *GlyphLookupMatrix = (kbts_u32 *)OutData; For kbts__os2 that messes with the byteswap reads. Using memcpy internally fixes it, optimized to direct access: static void kbts__ByteSwapArray16Unchecked(void *Array_, kbts_un Count) { kbts_u8 _Bytes = (kbts_u8 )Array; KBTS__FOR(It, 0, Count) { kbts_u16 V; KBTS_MEMCPY(&V, Bytes + It_2, 2); V = kbts__ByteSwap16(V); KBTS_MEMCPY(Bytes + It_2, &V, 2); } } For the signed shift, 1 << 31 is UB, just 1u << 31. I forked and put up my modifications for more reference, fixed all the traps I've ran into so far: https://github.com/Pesky01/kb

This is a good starting point, but I'm afraid it doesn't address the more fundamental issue I pointed out above.

I have fonts that put u32s, and arrays of u32s, at non-multiple-of-4 offsets within the file/their respective tables, and, as far as I know, there is nothing stopping any of the offsets in a font file from being the wrong alignment for the data it points to.

If we want to fully avoid this kind of UB (do you have any target platforms for which this is an issue?), I think we either need our own font representation or we need to use ReadXXXUnaligned whenever we access anything in the font.

With that said, in practice, maybe this is only an issue with u32s, and maybe there are few enough of them that always accessing them explicitly unaligned is not too bad of a patch.

Ahh, yeah doing unaligned read everywhere is a bit annoying. I agree just the u32 handling is sufficient so all even offsets are fine. For u16, you'd need a table at an odd offset (violating checksum) or an odd length field before u16?

The crash is writing to LookupSubtableIndexOffsets in the output blob, not reading packed font data. Since we control this layout, aligning before the cast fixes it: OutData = KBTS__ALIGN_POINTER(char, OutData, KBTS_ALIGNOF(kbts_u32)); kbts_u32 *GlyphLookupMatrix = (kbts_u32 *)OutData; For kbts__os2 that messes with the byteswap reads. Using memcpy internally fixes it, optimized to direct access: static void kbts__ByteSwapArray16Unchecked(void *Array_, kbts_un Count) { kbts_u8 _Bytes = (kbts_u8 )Array; KBTS__FOR(It, 0, Count) { kbts_u16 V; KBTS_MEMCPY(&V, Bytes + It_2, 2); V = kbts__ByteSwap16(V); KBTS_MEMCPY(Bytes + It_2, &V, 2); } } For the signed shift, 1 << 31 is UB, just 1u << 31. I forked and put up my modifications for more reference, fixed all the traps I've ran into so far: https://github.com/Pesky01/kb

This is a good starting point, but I'm afraid it doesn't address the more fundamental issue I pointed out above.

I have fonts that put u32s, and arrays of u32s, at non-multiple-of-4 offsets within the file/their respective tables, and, as far as I know, there is nothing stopping any of the offsets in a font file from being the wrong alignment for the data it points to.

If we want to fully avoid this kind of UB (do you have any target platforms for which this is an issue?), I think we either need our own font representation or we need to use ReadXXXUnaligned whenever we access anything in the font.

With that said, in practice, maybe this is only an issue with u32s, and maybe there are few enough of them that always accessing them explicitly unaligned is not too bad of a patch.

Ahh, yeah doing unaligned read everywhere is a bit annoying. I agree just the u32 handling is sufficient so all even offsets are fine. For u16, you'd need a table at an odd offset (violating checksum) or an odd length field before u16?

The odd offset on a u16 did happen in one of my test fonts. Fixing this specific one wasn't a problem, but we'll have to keep introducing unaligned reads if we find annoying fonts that have weird offsets.

In practice, though, the generated code looks fine (on x86_64) when using compile-time-sized memcpy, so that's a relief.

Thanks for nudging me to fix this, by the way. It's not exactly the most fun part of C programming.

The odd offset on a u16 did happen in one of my test fonts. Fixing this specific one wasn't a problem, but we'll have to keep introducing unaligned reads if we find annoying fonts that have weird offsets.

In practice, though, the generated code looks fine (on x86_64) when using compile-time-sized memcpy, so that's a relief.

Thanks for nudging me to fix this, by the way. It's not exactly the most fun part of C programming.

If you defined a __attribute__((packed)) struct { uint16_t v; } then the compiler is obliged to make that v be fine with unaligned access.

If you defined a __attribute__((packed)) struct { uint16_t v; } then the compiler is obliged to make that v be fine with unaligned access.

This is not exhaustive, but hopefully 9e2b6a2 covers most cases.

This fixes at least the issues I've seen so far. Thank you!!

This is not exhaustive, but hopefully 9e2b6a2 covers most cases.

This fixes at least the issues I've seen so far. Thank you!!

Updated to the new v2.09, I only receive the 1st crash still (kbts_ShapePushFontFromMemory), which is just because I have to manually align the embedded font to 4-bytes from the Zig side and it's fine, but the library can still handle that case.

Updated to the new v2.09, I only receive the 1st crash still (kbts_ShapePushFontFromMemory), which is just because I have to manually align the embedded font to 4-bytes from the Zig side and it's fine, but the library can still handle that case.

If you defined a __attribute__((packed)) struct { uint16_t v; } then the compiler is obliged to make that v be fine with unaligned access.

Good to know, thanks!

Since raw u16 arrays can be unaligned, too, I ended up adding explicit unaligned loads everywhere anyway.

If you defined a __attribute__((packed)) struct { uint16_t v; } then the compiler is obliged to make that v be fine with unaligned access.

Good to know, thanks!

Since raw u16 arrays can be unaligned, too, I ended up adding explicit unaligned loads everywhere anyway.

At commit cc63806 (v2.28e), a few misaligned u16 accesses are still there. Apple clang 21, -fsanitize=undefined, only loading the font with kbts_FontFromMemory:

Fonts that hit them, all from HarfBuzz's tests:

• cmap lines: 6991b13ce889466be6de3f66e891de2bc0f117ee.ttf (a normal CFF font, used in language-tags.tests)
• GSUB lines: 36f8e7f3b4f54efaa6bfd8225893022dc0041915.ttf and ecc344a5fa31c4d69350231bb3b96cc693646089.ttf (malformed fonts)

kb_text_shape.h:28094:37: runtime error: load of misaligned address 0x61f0000002b3 for type 'unsigned short', which requires 2 byte alignment
kb_text_shape.h:28100:21: runtime error: store to misaligned address 0x61f0000002bb for type 'unsigned short', which requires 2 byte alignment
kb_text_shape.h:17402:13: runtime error: load of misaligned address 0x00010c66c0b9 for type 'unsigned short', which requires 2 byte alignment

Under Zig's default sanitizer (trap mode), the first font is enough to crash a program in kbts_FontFromMemory.

(Found with help from Claude, with a harness that runs HarfBuzz's shaping test suite through kb and through HarfBuzz. I checked these by hand.)

https://github.com/JimmyLefevre/kb/commit/a936e1acd4088b917a1322341e057a5af399fe91 should hopefully fix all unaligned accesses.