Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zlib-ng: avoid suppressing WD4242 and WD4244 #105433

Merged
merged 19 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved

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");
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
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);

carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
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);
Comment on lines +61 to +63
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to restart the CI for this. I can either fix this in a separate PR, or we can just leave it as is:

  • One assert is not needed
  • The whole argument could be wrapped in parenthesis to convert the subtraction.
Suggested change
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);
Assert(s->strstart <= UINT16_MAX, "strstart-1 should fit in uint16_t");
check_match(s, (Pos)s->strstart, (Pos)(s->strstart - 1), match_len);

I'll make sure to submit the upstream PR with this fix though.


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
Loading