undefined behavior in bitstream.c

[andrewrk]

Upstream appears to be effectively dead, but there's undefined behavior happening here:

Illegal instruction at address 0x4ace666
/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/bitstream.c:181:50: 0x4ace666 in putbits2 (/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/bitstream.c)
        bs->buf[bs->buf_byte_idx] |= ((val >> j) << bs->buf_bit_idx);
                                                 ^
/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/bitstream.c:627:9: 0x4ad1f9a in Huffmancode (/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/bitstream.c)
        putbits2(gfc, (int)ext, xbits);
        ^
/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/bitstream.c:649:12: 0x4acf8ef in ShortHuffmancodebits (/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/bitstream.c)
    bits = Huffmancode(gfc, gi->table_select[0], 0, region1Start, gi);
           ^
/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/bitstream.c:718:34: 0x4aca7d5 in writeMainData (/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/bitstream.c)
                    data_bits += ShortHuffmancodebits(gfc, gi);
                                 ^
/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/bitstream.c:932:13: 0x4ac41b1 in format_bitstream (/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/bitstream.c)
    bits += writeMainData(gfc);
            ^
/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/encoder.c:544:12: 0x4b86665 in lame_encode_mp3_frame (/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/encoder.c)
    (void) format_bitstream(gfc);
           ^
/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/lame.c:1793:19: 0x4a4daa3 in lame_encode_buffer_sample_t (/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/lame.c)
            ret = lame_encode_mp3_frame(gfc, mfbuf[0], mfbuf[1], mp3buf, buf_size);
                  ^
/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/lame.c:1904:20: 0x4a43b23 in lame_encode_buffer_template (/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/lame.c)
            return lame_encode_buffer_sample_t(gfc, nsamples, mp3buf, mp3buf_size);
                   ^
/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/lame.c:1915:12: 0x4a43903 in lame_encode_buffer (/home/andy/.cache/zig/p/1220a15af8fd1628f0adfeb43782c49cdbe0ea8d45922f8f23772a2a4d0e2df7e072/libmp3lame/lame.c)
    return lame_encode_buffer_template(gfp, pcm_l, pcm_r, nsamples, mp3buf, mp3buf_size, pcm_short_type, 1, 1.0);
           ^
/home/andy/.cache/zig/p/1220e5bb0972b1f7e009566ddcf705d0b5ba049a4616f0a65fd97295e64fe0b3022a/libavcodec/libmp3lame.c:210:13: 0x4a2719d in mp3lame_encode_frame (/home/andy/.cache/zig/p/1220e5bb0972b1f7e009566ddcf705d0b5ba049a4616f0a65fd97295e64fe0b3022a/libavcodec/libmp3lame.c)
            ENCODE_BUFFER(lame_encode_buffer, int16_t, frame->data);
            ^
/home/andy/.cache/zig/p/1220e5bb0972b1f7e009566ddcf705d0b5ba049a4616f0a65fd97295e64fe0b3022a/libavcodec/encode.c:264:11: 0x39fcfda in ff_encode_encode_cb (/home/andy/.cache/zig/p/1220e5bb0972b1f7e009566ddcf705d0b5ba049a4616f0a65fd97295e64fe0b3022a/libavcodec/encode.c)
    ret = codec->cb.encode(avctx, avpkt, frame, got_packet);
          ^
/home/andy/.cache/zig/p/1220e5bb0972b1f7e009566ddcf705d0b5ba049a4616f0a65fd97295e64fe0b3022a/libavcodec/encode.c:350:15: 0x3a07fbc in encode_simple_internal (/home/andy/.cache/zig/p/1220e5bb0972b1f7e009566ddcf705d0b5ba049a4616f0a65fd97295e64fe0b3022a/libavcodec/encode.c)
        ret = ff_encode_encode_cb(avctx, avpkt, frame, &got_packet);
              ^
/home/andy/.cache/zig/p/1220e5bb0972b1f7e009566ddcf705d0b5ba049a4616f0a65fd97295e64fe0b3022a/libavcodec/encode.c:364:15: 0x3a0774b in encode_simple_receive_packet (/home/andy/.cache/zig/p/1220e5bb0972b1f7e009566ddcf705d0b5ba049a4616f0a65fd97295e64fe0b3022a/libavcodec/encode.c)
        ret = encode_simple_internal(avctx, avpkt);
              ^
/home/andy/.cache/zig/p/1220e5bb0972b1f7e009566ddcf705d0b5ba049a4616f0a65fd97295e64fe0b3022a/libavcodec/encode.c:398:15: 0x3a00166 in encode_receive_packet_internal (/home/andy/.cache/zig/p/1220e5bb0972b1f7e009566ddcf705d0b5ba049a4616f0a65fd97295e64fe0b3022a/libavcodec/encode.c)
        ret = encode_simple_receive_packet(avctx, avpkt);
              ^
/home/andy/.cache/zig/p/1220e5bb0972b1f7e009566ddcf705d0b5ba049a4616f0a65fd97295e64fe0b3022a/libavcodec/encode.c:541:15: 0x39fe627 in avcodec_send_frame (/home/andy/.cache/zig/p/1220e5bb0972b1f7e009566ddcf705d0b5ba049a4616f0a65fd97295e64fe0b3022a/libavcodec/encode.c)
        ret = encode_receive_packet_internal(avctx, avci->buffer_pkt);
              ^

[kavika13]

Would we want to try to fix the UB locally? If so, I might consider doing that and submitting a PR.

Is that going too far for such a repackaged lib? Or would it be welcome due to the special case of upstream being somewhat abandoned?

Right now I've had to do some hackery to work around this when using this dependency in my project:

const libmp3lame = b.dependency("libmp3lame", .{
    .target = target,
    .optimize = optimize,
});

const libmp3lame_artifact = libmp3lame.artifact("mp3lame");

for (libmp3lame_artifact.root_module.link_objects.items) |link_object| {
    switch (std.meta.activeTag(link_object)) {
        .c_source_files => {
            const new_flags = std.mem.concat(b.allocator, []const u8, &.{
                link_object.c_source_files.flags,
                &.{
                    "-fno-sanitize=undefined",
                },
            }) catch @panic("OOM");
            link_object.c_source_files.flags = new_flags;
        },
        else => {},
    }
}

... which works, but fixing the UB might be nice too.