Skip to content

Commit

Permalink
liblzma: Remove crc_attr_no_sanitize_address
Browse files Browse the repository at this point in the history
It's not enough to silence the address sanitizer. Also memory and
thread sanitizers would need to be silenced. They, at least currently,
aren't smart enough to see that the extra bytes are discarded from
the xmm registers by later instructions.

Valgrind is smarter, possibly because this kind of code isn't weird
to write in assembly. Agner Fog's optimizing_assembly.pdf even mentions
this idea of doing an aligned read and then discarding the extra
bytes. The sanitizers don't instrument assembly code but Valgrind
checks all code.

It's better to change the implementation to avoid the sanitization
attributes which also look scary in the code. (Somehow they can look
more scary than __asm__ which is implictly unsanitized.)

See also:
#112
#122
  • Loading branch information
Larhzu committed Jun 16, 2024
1 parent ead4d15 commit f99a7be
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 12 deletions.
9 changes: 0 additions & 9 deletions src/liblzma/check/crc_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,6 @@
#endif


// CRC CLMUL code needs this because accessing input buffers that aren't
// aligned to the vector size will inherently trip the address sanitizer.
#if lzma_has_attribute(__no_sanitize_address__)
# define crc_attr_no_sanitize_address \
__attribute__((__no_sanitize_address__))
#else
# define crc_attr_no_sanitize_address
#endif

// Keep this in sync with changes to crc32_arm64.h
#if defined(_WIN32) || defined(HAVE_GETAUXVAL) \
|| defined(HAVE_ELF_AUX_INFO) \
Expand Down
3 changes: 0 additions & 3 deletions src/liblzma/check/crc_x86_clmul.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@


crc_attr_target
crc_attr_no_sanitize_address
static lzma_always_inline void
crc_simd_body(const uint8_t *buf, const size_t size, __m128i *v0, __m128i *v1,
const __m128i vfold16, const __m128i initial_crc)
Expand Down Expand Up @@ -243,7 +242,6 @@ calc_hi(uint64_t p, uint64_t a, int n)
#ifdef BUILDING_CRC32_CLMUL

crc_attr_target
crc_attr_no_sanitize_address
static uint32_t
crc32_arch_optimized(const uint8_t *buf, size_t size, uint32_t crc)
{
Expand Down Expand Up @@ -333,7 +331,6 @@ calc_hi(uint64_t poly, uint64_t a)
#endif

crc_attr_target
crc_attr_no_sanitize_address
static uint64_t
crc64_arch_optimized(const uint8_t *buf, size_t size, uint64_t crc)
{
Expand Down

0 comments on commit f99a7be

Please sign in to comment.