The .gnu_debuglink section contains CRC32 checksum of debug information file. When LLVM is compiled without zlib support this checksum is completely ignored, which is error prone. This patch implements simple CRC32 calculation algorithm.
Details
Diff Detail
Event Timeline
include/llvm/Support/CRC.h | ||
---|---|---|
22 | I'm not sure if oss-fuzz builds llvm with zlib or not, but if it does, |
lib/DebugInfo/Symbolize/Symbolize.cpp | ||
---|---|---|
167 | What about always using non-zlib implementation? |
include/llvm/Support/CRC.h | ||
---|---|---|
22 | LLVM_ENABLE_ZLIB is ON by default, so whether LLVM is built with zlib support or not depends on HAVE_ZLIB_H. | |
lib/DebugInfo/Symbolize/Symbolize.cpp | ||
167 | zlib implementation is most likely faster, especially with -DBYFOUR. Also somebody may be using llvm::zlib::crc32 |
include/llvm/Support/CRC.h | ||
---|---|---|
22 | I guess this is a question of naming. I'd say: |
I'd recomment copying the version from libarchive (https://github.com/libarchive/libarchive/blob/master/libarchive/archive_crc32.h):
- don't bother with the 1KB table in both binary and source, just recompute it on the first use.
- at least in the past unrolling the inner loop somewhat helped a lot
I wouldn't really fuzz test this, just build a set of test vector and be done.
don't bother with the 1KB table in both binary and source, just recompute it on the first use
Is single threaded usage guaranteed?
Is there really any good case to build LLVM without zlib? Maybe it'd be better to just require zlib unconditionally (or mark building it really unsupported) rather than bundling extra algorithms that will be used only conditionally (until someone mistakenly uses it unconditionally, and LLVM would start using different CRC32 impls in different places) like this.
lib/Support/CMakeLists.txt | ||
---|---|---|
78 | Shouldn't this be only conditionally compiled if zlib isn't available? It seems wrong to have the lookup table twice. |
It doesn't matter, at most a single memory fence is necessary.
Yep. However, you don't have it in your code. The volatile int crc_tbl_inited prevents compiler optimizations but doesn't prevent out-of-order execution as far as I know.
Shouldn't this be only conditionally compiled if zlib isn't available?
How about putting the call to zlib::crc32 inside getCRC32 and using getCRC32 from Symbolize.cpp and other places?
I still feel this code should be in an #ifndef HAVE_ZLIB with the #else branch forwarding the call into zlib.
include/llvm/Support/CRC.h | ||
---|---|---|
22 | /// |
include/llvm/Support/CRC.h | ||
---|---|---|
23 | Why not just call this llvm::crc32()? |
I'm not sure if oss-fuzz builds llvm with zlib or not, but if it does,
this is a *great* fuzzing target, to cross-check with the zlib output.