Skip to content

Commit

Permalink
zlib-ng: avoid suppressing WD4242 and WD4244 (#105433)
Browse files Browse the repository at this point in the history
WD4242 and WD4244 are compiler warnings that should not be suppressed because the warn about possible loss of data.

WD4242 shows up in zlib-ng/arch/*/slide_hash*.c files and comes from the arguments passed to the slide_hash_chain method.
WD4244 happens in Windows when building in Debug configuration, in various zlib-ng/deflate*.c files, and comes from the arguments passed to the check_match method.

Fixed by:

- Adding asserts to verify the values are below the maximum allowed for their type.
- Casting them the proper type before passing them as arguments to their methods.
- Removing the WD suppressions, which unfortunately also propagated to other unrelated cmake files.
- Fixed a similar loss of data error in an unrelated mono file where the warning suppression was propagated due to this.
  • Loading branch information
carlossanlop authored and directhex committed Jul 26, 2024
1 parent 28ad257 commit 511c9f3
Show file tree
Hide file tree
Showing 13 changed files with 25 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/mono/mono/metadata/class-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -1467,7 +1467,7 @@ mono_method_has_unmanaged_callers_only_attribute (MonoMethod *method);

typedef struct _MonoVarianceSearchTableEntry {
MonoClass *klass;
guint16 offset;
int offset;
} MonoVarianceSearchTableEntry;

MonoVarianceSearchTableEntry *
Expand Down
2 changes: 2 additions & 0 deletions src/native/external/zlib-ng-version.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,5 @@ We have removed the following folders from our local copy as these files are not
- zlib-ng/docs/
- zlib-ng/test/
- zlib-ng/arch/s390/self-hosted-builder/

Apply https://github.com/dotnet/runtime/pull/105433.patch
9 changes: 3 additions & 6 deletions src/native/external/zlib-ng.cmake
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# IMPORTANT: do not use add_compile_options(), add_definitions() or similar functions here since it will leak to the including projects

include(FetchContent)

FetchContent_Declare(
Expand All @@ -12,12 +14,6 @@ set(Z_PREFIX ON)
# TODO: Turn back on when Linux kernels with proper RISC-V extension detection (>= 6.5) are more commonplace
set(WITH_RVV OFF)

add_compile_options($<$<COMPILE_LANG_AND_ID:C,Clang,AppleClang>:-Wno-unused-command-line-argument>) # clang : error : argument unused during compilation: '-fno-semantic-interposition'
add_compile_options($<$<COMPILE_LANG_AND_ID:C,Clang,AppleClang>:-Wno-logical-op-parentheses>) # place parentheses around the '&&' expression to silence this warning
add_compile_options($<$<COMPILE_LANG_AND_ID:C,MSVC>:/wd4127>) # warning C4127: conditional expression is constant
add_compile_options($<$<COMPILE_LANG_AND_ID:C,MSVC>:/wd4242>) # 'function': conversion from 'unsigned int' to 'Pos', possible loss of data, in various deflate_*.c files
add_compile_options($<$<COMPILE_LANG_AND_ID:C,MSVC>:/wd4244>) # 'function': conversion from 'unsigned int' to 'Pos', possible loss of data, in various deflate_*.c files

# 'aligned_alloc' is not available in browser/wasi, yet it is set by zlib-ng/CMakeLists.txt.
if (CLR_CMAKE_TARGET_BROWSER OR CLR_CMAKE_TARGET_WASI)
set(HAVE_ALIGNED_ALLOC FALSE CACHE BOOL "have aligned_alloc" FORCE)
Expand All @@ -31,6 +27,7 @@ set(SKIP_INSTALL_ALL OFF)
set_property(DIRECTORY ${CMAKE_CURRENT_LIST_DIR}/zlib-ng PROPERTY MSVC_WARNING_LEVEL 3) # Set the MSVC warning level for all zlib-ng targets to 3.
target_compile_options(zlib PRIVATE $<$<COMPILE_LANG_AND_ID:C,Clang,AppleClang>:-Wno-unused-command-line-argument>) # Make sure MacOS respects ignoring unused CLI arguments
target_compile_options(zlib PRIVATE $<$<COMPILE_LANG_AND_ID:C,Clang,AppleClang>:-Wno-logical-op-parentheses>) # place parentheses around the '&&' expression to silence this warning
target_compile_options(zlib PRIVATE $<$<COMPILE_LANG_AND_ID:C,MSVC>:/wd4127>) # warning C4127: conditional expression is constant
target_compile_options(zlib PRIVATE $<$<COMPILE_LANG_AND_ID:C,MSVC>:/guard:cf>) # Enable CFG always for zlib-ng so we don't need to build two flavors.

set_target_properties(zlib PROPERTIES DEBUG_POSTFIX "") # Workaround: zlib's debug lib name is zlibd.lib
3 changes: 2 additions & 1 deletion src/native/external/zlib-ng/arch/arm/slide_hash_armv6.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ static inline void slide_hash_chain(Pos *table, uint32_t entries, uint16_t wsize
}

Z_INTERNAL void slide_hash_armv6(deflate_state *s) {
unsigned int wsize = s->w_size;
Assert(s->w_size <= UINT16_MAX, "w_size should fit in uint16_t");
uint16_t wsize = (uint16_t)s->w_size;

slide_hash_chain(s->head, HASH_SIZE, wsize);
slide_hash_chain(s->prev, wsize, wsize);
Expand Down
3 changes: 2 additions & 1 deletion src/native/external/zlib-ng/arch/arm/slide_hash_neon.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ static inline void slide_hash_chain(Pos *table, uint32_t entries, uint16_t wsize
}

Z_INTERNAL void slide_hash_neon(deflate_state *s) {
unsigned int wsize = s->w_size;
Assert(s->w_size <= UINT16_MAX, "w_size should fit in uint16_t");
uint16_t wsize = (uint16_t)s->w_size;

slide_hash_chain(s->head, HASH_SIZE, wsize);
slide_hash_chain(s->prev, wsize, wsize);
Expand Down
3 changes: 2 additions & 1 deletion src/native/external/zlib-ng/arch/power/slide_ppc_tpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ static inline void slide_hash_chain(Pos *table, uint32_t entries, uint16_t wsize
}

void Z_INTERNAL SLIDE_PPC(deflate_state *s) {
uint16_t wsize = s->w_size;
Assert(s->w_size <= UINT16_MAX, "w_size should fit in uint16_t");
uint16_t wsize = (uint16_t)s->w_size;

slide_hash_chain(s->head, HASH_SIZE, wsize);
slide_hash_chain(s->prev, wsize, wsize);
Expand Down
1 change: 1 addition & 0 deletions src/native/external/zlib-ng/arch/riscv/slide_hash_rvv.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ static inline void slide_hash_chain(Pos *table, uint32_t entries, uint16_t wsize
}

Z_INTERNAL void slide_hash_rvv(deflate_state *s) {
Assert(s->w_size <= UINT16_MAX, "w_size should fit in uint16_t");
uint16_t wsize = (uint16_t)s->w_size;

slide_hash_chain(s->head, HASH_SIZE, wsize);
Expand Down
1 change: 1 addition & 0 deletions src/native/external/zlib-ng/arch/x86/slide_hash_avx2.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ static inline void slide_hash_chain(Pos *table, uint32_t entries, const __m256i
}

Z_INTERNAL void slide_hash_avx2(deflate_state *s) {
Assert(s->w_size <= UINT16_MAX, "w_size should fit in uint16_t");
uint16_t wsize = (uint16_t)s->w_size;
const __m256i ymm_wsize = _mm256_set1_epi16((short)wsize);

Expand Down
1 change: 1 addition & 0 deletions src/native/external/zlib-ng/arch/x86/slide_hash_sse2.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ static inline void slide_hash_chain(Pos *table0, Pos *table1, uint32_t entries0,
}

Z_INTERNAL void slide_hash_sse2(deflate_state *s) {
Assert(s->w_size <= UINT16_MAX, "w_size should fit in uint16_t");
uint16_t wsize = (uint16_t)s->w_size;
const __m128i xmm_wsize = _mm_set1_epi16((short)wsize);

Expand Down
4 changes: 3 additions & 1 deletion src/native/external/zlib-ng/deflate_fast.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ Z_INTERNAL block_state deflate_fast(deflate_state *s, int flush) {
}

if (match_len >= WANT_MIN_MATCH) {
check_match(s, s->strstart, s->match_start, match_len);
Assert(s->strstart <= UINT16_MAX, "strstart should fit in uint16_t");
Assert(s->match_start <= UINT16_MAX, "match_start should fit in uint16_t");
check_match(s, (Pos)s->strstart, (Pos)s->match_start, match_len);

bflush = zng_tr_tally_dist(s, s->strstart - s->match_start, match_len - STD_MIN_MATCH);

Expand Down
3 changes: 2 additions & 1 deletion src/native/external/zlib-ng/deflate_quick.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ Z_INTERNAL block_state deflate_quick(deflate_state *s, int flush) {
if (UNLIKELY(match_len > STD_MAX_MATCH))
match_len = STD_MAX_MATCH;

check_match(s, s->strstart, hash_head, match_len);
Assert(s->strstart <= UINT16_MAX, "strstart should fit in uint16_t");
check_match(s, (Pos)s->strstart, hash_head, match_len);

zng_tr_emit_dist(s, static_ltree, static_dtree, match_len - STD_MIN_MATCH, (uint32_t)dist);
s->lookahead -= match_len;
Expand Down
4 changes: 3 additions & 1 deletion src/native/external/zlib-ng/deflate_rle.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ Z_INTERNAL block_state deflate_rle(deflate_state *s, int flush) {

/* Emit match if have run of STD_MIN_MATCH or longer, else emit literal */
if (match_len >= STD_MIN_MATCH) {
check_match(s, s->strstart, s->strstart - 1, match_len);
Assert(s->strstart <= UINT16_MAX, "strstart should fit in uint16_t");
Assert(s->match_start <= UINT16_MAX, "match_start should fit in uint16_t");
check_match(s, (Pos)s->strstart, (Pos)s->strstart - 1, match_len);

bflush = zng_tr_tally_dist(s, 1, match_len - STD_MIN_MATCH);

Expand Down
3 changes: 2 additions & 1 deletion src/native/external/zlib-ng/deflate_slow.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ Z_INTERNAL block_state deflate_slow(deflate_state *s, int flush) {
unsigned int max_insert = s->strstart + s->lookahead - STD_MIN_MATCH;
/* Do not insert strings in hash table beyond this. */

check_match(s, s->strstart-1, s->prev_match, s->prev_length);
Assert((s->strstart-1) <= UINT16_MAX, "strstart-1 should fit in uint16_t");
check_match(s, (Pos)(s->strstart-1), s->prev_match, s->prev_length);

bflush = zng_tr_tally_dist(s, s->strstart -1 - s->prev_match, s->prev_length - STD_MIN_MATCH);

Expand Down

0 comments on commit 511c9f3

Please sign in to comment.