Skip to content

Commit 80fffd7

Browse files
Dead2Dead2
committed
Since we long ago make unaligned reads safe (by using memcpy or intrinsics),
it is time to replace the UNALIGNED_OK checks that have since really only been used to select the optimal comparison sizes for the arch instead.
1 parent 43d74a2 commit 80fffd7

17 files changed

Lines changed: 92 additions & 108 deletions

‎.github/workflows/cmake.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,11 @@ jobs:
8181
build-src-dir: ../zlib-ng/test/add-subdirectory-project
8282
readonly-project-dir: true
8383

84-
- name: Ubuntu GCC -O1 No Unaligned UBSAN
84+
- name: Ubuntu GCC -O1 UBSAN
8585
os: ubuntu-latest
8686
compiler: gcc
8787
cxx-compiler: g++
88-
cmake-args: -DWITH_UNALIGNED=OFF -DWITH_SANITIZER=Undefined
88+
cmake-args: -DWITH_SANITIZER=Undefined
8989
codecov: ubuntu_gcc_o1
9090
cflags: -O1
9191

‎CMakeLists.txt‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ option(WITH_MAINTAINER_WARNINGS "Build with project maintainer warnings" OFF)
9393
option(WITH_CODE_COVERAGE "Enable code coverage reporting" OFF)
9494
option(WITH_INFLATE_STRICT "Build with strict inflate distance checking" OFF)
9595
option(WITH_INFLATE_ALLOW_INVALID_DIST "Build with zero fill for inflate invalid distances" OFF)
96-
option(WITH_UNALIGNED "Support unaligned reads on platforms that support it" ON)
9796

9897
set(ZLIB_SYMBOL_PREFIX "" CACHE STRING "Give this prefix to all publicly exported symbols.
9998
Useful when embedding into a larger library.
@@ -147,7 +146,6 @@ mark_as_advanced(FORCE
147146
WITH_RVV
148147
WITH_INFLATE_STRICT
149148
WITH_INFLATE_ALLOW_INVALID_DIST
150-
WITH_UNALIGNED
151149
INSTALL_UTILS
152150
)
153151

@@ -336,12 +334,6 @@ if(NOT WITH_NATIVE_INSTRUCTIONS)
336334
endforeach()
337335
endif()
338336

339-
# Set architecture alignment requirements
340-
if(NOT WITH_UNALIGNED)
341-
add_definitions(-DNO_UNALIGNED)
342-
message(STATUS "Unaligned reads manually disabled")
343-
endif()
344-
345337
# Apply warning compiler flags
346338
if(WITH_MAINTAINER_WARNINGS)
347339
add_compile_options(${WARNFLAGS} ${WARNFLAGS_MAINTAINER} ${WARNFLAGS_DISABLE})

‎README.md‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ Features
2525
* Compare256 implementations using SSE2, AVX2, Neon, POWER9 & RVV
2626
* Inflate chunk copying using SSE2, SSSE3, AVX, Neon & VSX
2727
* Support for hardware-accelerated deflate using IBM Z DFLTCC
28-
* Unaligned memory read/writes and large bit buffer improvements
28+
* Safe unaligned memory read/writes and large bit buffer improvements
2929
* Includes improvements from Cloudflare and Intel forks
3030
* Configure, CMake, and NMake build system support
3131
* Comprehensive set of CMake unit tests
@@ -213,7 +213,6 @@ Advanced Build Options
213213
| WITH_CRC32_VX | --without-crc32-vx | Build with vectorized CRC32 on IBM Z | ON |
214214
| WITH_DFLTCC_DEFLATE | --with-dfltcc-deflate | Build with DFLTCC intrinsics for compression on IBM Z | OFF |
215215
| WITH_DFLTCC_INFLATE | --with-dfltcc-inflate | Build with DFLTCC intrinsics for decompression on IBM Z | OFF |
216-
| WITH_UNALIGNED | --without-unaligned | Allow optimizations that use unaligned reads if safe on current arch| ON |
217216
| WITH_INFLATE_STRICT | | Build with strict inflate distance checking | OFF |
218217
| WITH_INFLATE_ALLOW_INVALID_DIST | | Build with zero fill for inflate invalid distances | OFF |
219218
| INSTALL_UTILS | | Copy minigzip and minideflate during install | OFF |

‎arch/generic/compare256_c.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ Z_INTERNAL uint32_t compare256_c(const uint8_t *src0, const uint8_t *src1) {
5757

5858
#include "match_tpl.h"
5959

60-
#if defined(UNALIGNED_OK) && BYTE_ORDER == LITTLE_ENDIAN
60+
#if BYTE_ORDER == LITTLE_ENDIAN && OPTIMAL_CMP >= 32
6161
/* 16-bit unaligned integer comparison */
6262
static inline uint32_t compare256_unaligned_16_static(const uint8_t *src0, const uint8_t *src1) {
6363
uint32_t len = 0;
@@ -138,8 +138,8 @@ Z_INTERNAL uint32_t compare256_unaligned_32(const uint8_t *src0, const uint8_t *
138138

139139
#endif
140140

141-
#if defined(UNALIGNED64_OK) && defined(HAVE_BUILTIN_CTZLL)
142-
/* UNALIGNED64_OK, 64-bit integer comparison */
141+
#if defined(HAVE_BUILTIN_CTZLL) && OPTIMAL_CMP >= 64
142+
/* 64-bit integer comparison */
143143
static inline uint32_t compare256_unaligned_64_static(const uint8_t *src0, const uint8_t *src1) {
144144
uint32_t len = 0;
145145

‎arch/generic/generic_functions.h‎

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,13 @@ void inflate_fast_c(PREFIX3(stream) *strm, uint32_t start);
2828
uint32_t PREFIX(crc32_braid)(uint32_t crc, const uint8_t *buf, size_t len);
2929

3030
uint32_t compare256_c(const uint8_t *src0, const uint8_t *src1);
31-
#if defined(UNALIGNED_OK) && BYTE_ORDER == LITTLE_ENDIAN
32-
uint32_t compare256_unaligned_16(const uint8_t *src0, const uint8_t *src1);
31+
#if BYTE_ORDER == LITTLE_ENDIAN && OPTIMAL_CMP >= 32
32+
uint32_t compare256_unaligned_16(const uint8_t *src0, const uint8_t *src1);
3333
# ifdef HAVE_BUILTIN_CTZ
34-
uint32_t compare256_unaligned_32(const uint8_t *src0, const uint8_t *src1);
34+
uint32_t compare256_unaligned_32(const uint8_t *src0, const uint8_t *src1);
3535
# endif
36-
# if defined(UNALIGNED64_OK) && defined(HAVE_BUILTIN_CTZLL)
37-
uint32_t compare256_unaligned_64(const uint8_t *src0, const uint8_t *src1);
36+
# if defined(HAVE_BUILTIN_CTZLL) && OPTIMAL_CMP >= 64
37+
uint32_t compare256_unaligned_64(const uint8_t *src0, const uint8_t *src1);
3838
# endif
3939
#endif
4040

@@ -43,29 +43,24 @@ typedef void (*slide_hash_func)(deflate_state *s);
4343
void slide_hash_c(deflate_state *s);
4444

4545
uint32_t longest_match_c(deflate_state *const s, Pos cur_match);
46-
# if defined(UNALIGNED_OK) && BYTE_ORDER == LITTLE_ENDIAN
46+
uint32_t longest_match_slow_c(deflate_state *const s, Pos cur_match);
47+
#if BYTE_ORDER == LITTLE_ENDIAN && OPTIMAL_CMP >= 32
4748
uint32_t longest_match_unaligned_16(deflate_state *const s, Pos cur_match);
48-
# ifdef HAVE_BUILTIN_CTZ
49+
uint32_t longest_match_slow_unaligned_16(deflate_state *const s, Pos cur_match);
50+
# ifdef HAVE_BUILTIN_CTZ
4951
uint32_t longest_match_unaligned_32(deflate_state *const s, Pos cur_match);
50-
# endif
51-
# if defined(UNALIGNED64_OK) && defined(HAVE_BUILTIN_CTZLL)
52-
uint32_t longest_match_unaligned_64(deflate_state *const s, Pos cur_match);
53-
# endif
52+
uint32_t longest_match_slow_unaligned_32(deflate_state *const s, Pos cur_match);
5453
# endif
55-
56-
uint32_t longest_match_slow_c(deflate_state *const s, Pos cur_match);
57-
# if defined(UNALIGNED_OK) && BYTE_ORDER == LITTLE_ENDIAN
58-
uint32_t longest_match_slow_unaligned_16(deflate_state *const s, Pos cur_match);
59-
uint32_t longest_match_slow_unaligned_32(deflate_state *const s, Pos cur_match);
60-
# ifdef UNALIGNED64_OK
54+
# if defined(HAVE_BUILTIN_CTZLL) && OPTIMAL_CMP >= 64
55+
uint32_t longest_match_unaligned_64(deflate_state *const s, Pos cur_match);
6156
uint32_t longest_match_slow_unaligned_64(deflate_state *const s, Pos cur_match);
62-
# endif
6357
# endif
58+
#endif
6459

6560

6661
// Select generic implementation for longest_match, longest_match_slow, longest_match_slow functions.
67-
#if defined(UNALIGNED_OK) && BYTE_ORDER == LITTLE_ENDIAN
68-
# if defined(UNALIGNED64_OK) && defined(HAVE_BUILTIN_CTZLL)
62+
#if BYTE_ORDER == LITTLE_ENDIAN && OPTIMAL_CMP >= 32
63+
# if defined(HAVE_BUILTIN_CTZLL) && OPTIMAL_CMP >= 64
6964
# define longest_match_generic longest_match_unaligned_64
7065
# define longest_match_slow_generic longest_match_slow_unaligned_64
7166
# define compare256_generic compare256_unaligned_64

‎chunkset_tpl.h‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -227,17 +227,15 @@ static inline uint8_t* CHUNKMEMSET(uint8_t *out, uint8_t *from, unsigned len) {
227227
}
228228

229229
Z_INTERNAL uint8_t* CHUNKMEMSET_SAFE(uint8_t *out, uint8_t *from, unsigned len, unsigned left) {
230-
#if !defined(UNALIGNED64_OK)
231-
# if !defined(UNALIGNED_OK)
230+
#if OPTIMAL_CMP < 32
232231
static const uint32_t align_mask = 7;
233-
# else
232+
#elif OPTIMAL_CMP == 32
234233
static const uint32_t align_mask = 3;
235-
# endif
236234
#endif
237235

238236
len = MIN(len, left);
239237

240-
#if !defined(UNALIGNED64_OK)
238+
#if OPTIMAL_CMP < 64
241239
while (((uintptr_t)out & align_mask) && (len > 0)) {
242240
*out++ = *from++;
243241
--len;

‎cmake/detect-sanitizer.cmake‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ endmacro()
111111

112112
macro(add_undefined_sanitizer)
113113
set(known_checks
114+
alignment
114115
array-bounds
115116
bool
116117
bounds
@@ -137,10 +138,6 @@ macro(add_undefined_sanitizer)
137138
vptr
138139
)
139140

140-
# Only check for alignment sanitizer flag if unaligned access is not supported
141-
if(NOT WITH_UNALIGNED)
142-
list(APPEND known_checks alignment)
143-
endif()
144141
# Object size sanitizer has no effect at -O0 and produces compiler warning if enabled
145142
if(NOT CMAKE_C_FLAGS MATCHES "-O0")
146143
list(APPEND known_checks object-size)
@@ -153,12 +150,6 @@ macro(add_undefined_sanitizer)
153150
add_compile_options(-fsanitize=${supported_checks})
154151
add_link_options(-fsanitize=${supported_checks})
155152

156-
# Group sanitizer flag -fsanitize=undefined will automatically add alignment, even if
157-
# it is not in our sanitize flag list, so we need to explicitly disable alignment sanitizing.
158-
if(WITH_UNALIGNED)
159-
add_compile_options(-fno-sanitize=alignment)
160-
endif()
161-
162153
add_common_sanitizer_flags()
163154
else()
164155
message(STATUS "Undefined behavior sanitizer is not supported")

‎compare256_rle.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ static inline uint32_t compare256_rle_c(const uint8_t *src0, const uint8_t *src1
4242
return 256;
4343
}
4444

45-
#ifdef UNALIGNED_OK
45+
#if OPTIMAL_CMP >= 16
4646
/* 16-bit unaligned integer comparison */
4747
static inline uint32_t compare256_rle_unaligned_16(const uint8_t *src0, const uint8_t *src1) {
4848
uint32_t len = 0;
@@ -100,7 +100,7 @@ static inline uint32_t compare256_rle_unaligned_32(const uint8_t *src0, const ui
100100

101101
#endif
102102

103-
#if defined(UNALIGNED64_OK) && defined(HAVE_BUILTIN_CTZLL)
103+
#if defined(HAVE_BUILTIN_CTZLL) && OPTIMAL_CMP >= 64
104104
/* 64-bit unaligned integer comparison */
105105
static inline uint32_t compare256_rle_unaligned_64(const uint8_t *src0, const uint8_t *src1) {
106106
uint32_t src0_cmp32, len = 0;

‎configure‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ mandir=${mandir-'${prefix}/share/man'}
8787
shared_ext='.so'
8888
shared=1
8989
gzfileops=1
90-
unalignedok=1
9190
compat=0
9291
cover=0
9392
build32=0
@@ -164,7 +163,6 @@ case "$1" in
164163
echo ' [--warn] Enables extra compiler warnings' | tee -a configure.log
165164
echo ' [--debug] Enables extra debug prints during operation' | tee -a configure.log
166165
echo ' [--zlib-compat] Compiles for zlib-compatible API instead of zlib-ng API' | tee -a configure.log
167-
echo ' [--without-unaligned] Compiles without fast unaligned access' | tee -a configure.log
168166
echo ' [--without-gzfileops] Compiles without the gzfile parts of the API enabled' | tee -a configure.log
169167
echo ' [--without-optimizations] Compiles without support for optional instruction sets' | tee -a configure.log
170168
echo ' [--without-new-strategies] Compiles without using new additional deflate strategies' | tee -a configure.log
@@ -195,7 +193,6 @@ case "$1" in
195193
-s* | --shared | --enable-shared) shared=1; shift ;;
196194
-t | --static) shared=0; shift ;;
197195
--zlib-compat) compat=1; shift ;;
198-
--without-unaligned) unalignedok=0; shift ;;
199196
--without-gzfileops) gzfileops=0; shift ;;
200197
--cover) cover=1; shift ;;
201198
-3* | --32) build32=1; shift ;;
@@ -876,13 +873,6 @@ else
876873
PIC_TESTOBJG="\$(OBJG)"
877874
fi
878875

879-
# set architecture alignment requirements
880-
if test $unalignedok -eq 0; then
881-
CFLAGS="${CFLAGS} -DNO_UNALIGNED"
882-
SFLAGS="${SFLAGS} -DNO_UNALIGNED"
883-
echo "Unaligned reads manually disabled." | tee -a configure.log
884-
fi
885-
886876
# enable reduced memory configuration
887877
if test $reducedmem -eq 1; then
888878
echo "Configuring for reduced memory environment." | tee -a configure.log

‎deflate_rle.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010
#include "deflate_p.h"
1111
#include "functable.h"
1212

13-
#ifdef UNALIGNED_OK
14-
# if defined(UNALIGNED64_OK) && defined(HAVE_BUILTIN_CTZLL)
13+
#if OPTIMAL_CMP >= 32
14+
# if defined(HAVE_BUILTIN_CTZLL) && OPTIMAL_CMP >= 64
1515
# define compare256_rle compare256_rle_unaligned_64
1616
# elif defined(HAVE_BUILTIN_CTZ)
1717
# define compare256_rle compare256_rle_unaligned_32

0 commit comments

Comments
 (0)