[llvm] [LLVM] Add zstd and zlib compressed frame magic to LLVM magic (PR #222773)
James Henderson via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 17 09:02:16 PDT 2026
jh7370 wrote:
> > `identify_magic` feels like the wrong approach, because you're not dealing with arbitrary objects and you know that the data is compressed already.
>
> The format knows that the object is 'compressed', but it does not know which format was used. Rather than encode this in the object format, I was hoping I could just use the fact that `zstd` publishes magic bytes. The original plan was that we could tell if it's zstd, otherwise it's zlib, Presumably decompressing malformed zlib would return an error so I thought it was safe. `identify_magic` is the canonical place for detecting format magic, so it seemed appropriate.
>
> If you object to `zlib` I could remove that and go back to the earlier approach. Personally, I think doing this through magic detection and failing on malformed bytes is perfectly reasonable. We have the original size so it should be pretty much impossible to get a valid bytestream that both inflates without error and matches the size, so I don't see the need to complicate the binary format with what I consider redundant information.
>
> If you object to this being here I can just check the magic bytes directly in `OffloadBinary.cpp`, but this seemed like the better option because it's re-usable and other components might like to check.
I think you've misunderstood my suggestion. If you take a look at the [clang AST reader](https://github.com/llvm/llvm-project/blob/36763294ef639d8a6a4379d81da653dda1c20686/clang/lib/Serialization/ASTReader.cpp#L1932), it knows that data is compressed, but determines the compression type using the magic bytes. It doesn't use `identify_magic` however. My recommendation would be to extract the magic detection and subsequent decompression used there into a new method within the `llvm::compression` namespace. This would avoid duplicating the logic there, without having to add zstd/zlib detection to the more generic `identify_magic` without a more general need for it.
https://github.com/llvm/llvm-project/pull/222773
More information about the llvm-commits
mailing list