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 12 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
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
From e2106cd3e5771690867d826e525e4ad80619d76b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20S=C3=A1nchez=20L=C3=B3pez?=
<1175054+carlossanlop@users.noreply.github.com>
Date: Wed, 24 Jul 2024 18:43:38 -0700
Subject: [PATCH 1/2] Add aserts and casts to slide_hash*

---
src/native/external/zlib-ng/arch/arm/slide_hash_armv6.c | 4 +++-
src/native/external/zlib-ng/arch/arm/slide_hash_neon.c | 4 +++-
src/native/external/zlib-ng/arch/power/slide_ppc_tpl.h | 4 +++-
src/native/external/zlib-ng/arch/riscv/slide_hash_rvv.c | 2 ++
src/native/external/zlib-ng/arch/x86/slide_hash_avx2.c | 2 ++
src/native/external/zlib-ng/arch/x86/slide_hash_sse2.c | 1 +
6 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/native/external/zlib-ng/arch/arm/slide_hash_armv6.c b/src/native/external/zlib-ng/arch/arm/slide_hash_armv6.c
index 0a2eeccf926..ef226fad5d4 100644
--- a/src/native/external/zlib-ng/arch/arm/slide_hash_armv6.c
+++ b/src/native/external/zlib-ng/arch/arm/slide_hash_armv6.c
@@ -7,6 +7,7 @@
#include "acle_intrins.h"
#include "../../zbuild.h"
#include "../../deflate.h"
+#include <assert.h>

/* SIMD version of hash_chain rebase */
static inline void slide_hash_chain(Pos *table, uint32_t entries, uint16_t wsize) {
@@ -39,7 +40,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);
+ uint16_t wsize = (uint16_t)s->w_size;

slide_hash_chain(s->head, HASH_SIZE, wsize);
slide_hash_chain(s->prev, wsize, wsize);
diff --git a/src/native/external/zlib-ng/arch/arm/slide_hash_neon.c b/src/native/external/zlib-ng/arch/arm/slide_hash_neon.c
index a96ca11799b..b9574a308e0 100644
--- a/src/native/external/zlib-ng/arch/arm/slide_hash_neon.c
+++ b/src/native/external/zlib-ng/arch/arm/slide_hash_neon.c
@@ -12,6 +12,7 @@
#include "neon_intrins.h"
#include "../../zbuild.h"
#include "../../deflate.h"
+#include <assert.h>

/* SIMD version of hash_chain rebase */
static inline void slide_hash_chain(Pos *table, uint32_t entries, uint16_t wsize) {
@@ -38,7 +39,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);
+ uint16_t wsize = (uint16_t)s->w_size;

slide_hash_chain(s->head, HASH_SIZE, wsize);
slide_hash_chain(s->prev, wsize, wsize);
diff --git a/src/native/external/zlib-ng/arch/power/slide_ppc_tpl.h b/src/native/external/zlib-ng/arch/power/slide_ppc_tpl.h
index 5c17e38fb31..44c61c27da1 100644
--- a/src/native/external/zlib-ng/arch/power/slide_ppc_tpl.h
+++ b/src/native/external/zlib-ng/arch/power/slide_ppc_tpl.h
@@ -6,6 +6,7 @@
#include <altivec.h>
#include "zbuild.h"
#include "deflate.h"
+#include <assert.h>

static inline void slide_hash_chain(Pos *table, uint32_t entries, uint16_t wsize) {
const vector unsigned short vmx_wsize = vec_splats(wsize);
@@ -24,7 +25,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);
+ uint16_t wsize = (uint16_t)s->w_size;

slide_hash_chain(s->head, HASH_SIZE, wsize);
slide_hash_chain(s->prev, wsize, wsize);
diff --git a/src/native/external/zlib-ng/arch/riscv/slide_hash_rvv.c b/src/native/external/zlib-ng/arch/riscv/slide_hash_rvv.c
index 1164e89ba25..2e758742373 100644
--- a/src/native/external/zlib-ng/arch/riscv/slide_hash_rvv.c
+++ b/src/native/external/zlib-ng/arch/riscv/slide_hash_rvv.c
@@ -10,6 +10,7 @@

#include "../../zbuild.h"
#include "../../deflate.h"
+#include <assert.h>

static inline void slide_hash_chain(Pos *table, uint32_t entries, uint16_t wsize) {
size_t vl;
@@ -25,6 +26,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);
uint16_t wsize = (uint16_t)s->w_size;

slide_hash_chain(s->head, HASH_SIZE, wsize);
diff --git a/src/native/external/zlib-ng/arch/x86/slide_hash_avx2.c b/src/native/external/zlib-ng/arch/x86/slide_hash_avx2.c
index 94fe10c7bf4..144b5672940 100644
--- a/src/native/external/zlib-ng/arch/x86/slide_hash_avx2.c
+++ b/src/native/external/zlib-ng/arch/x86/slide_hash_avx2.c
@@ -13,6 +13,7 @@
#include "../../deflate.h"

#include <immintrin.h>
+#include <assert.h>

static inline void slide_hash_chain(Pos *table, uint32_t entries, const __m256i wsize) {
table += entries;
@@ -31,6 +32,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);
uint16_t wsize = (uint16_t)s->w_size;
const __m256i ymm_wsize = _mm256_set1_epi16((short)wsize);

diff --git a/src/native/external/zlib-ng/arch/x86/slide_hash_sse2.c b/src/native/external/zlib-ng/arch/x86/slide_hash_sse2.c
index 5daac4a7398..04db335e3de 100644
--- a/src/native/external/zlib-ng/arch/x86/slide_hash_sse2.c
+++ b/src/native/external/zlib-ng/arch/x86/slide_hash_sse2.c
@@ -52,6 +52,7 @@ next_chain:
}

Z_INTERNAL void slide_hash_sse2(deflate_state *s) {
+ assert(s->w_size <= UINT16_MAX);
uint16_t wsize = (uint16_t)s->w_size;
const __m128i xmm_wsize = _mm_set1_epi16((short)wsize);

--
2.45.2.windows.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
From 51c39e062b7df96a789da1818c3e2446e9f60d64 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20S=C3=A1nchez=20L=C3=B3pez?=
<1175054+carlossanlop@users.noreply.github.com>
Date: Wed, 24 Jul 2024 18:44:08 -0700
Subject: [PATCH 2/2] Add asserts and casts to check_match

---
src/native/external/zlib-ng/deflate_fast.c | 4 +++-
src/native/external/zlib-ng/deflate_quick.c | 3 ++-
src/native/external/zlib-ng/deflate_rle.c | 4 +++-
src/native/external/zlib-ng/deflate_slow.c | 3 ++-
4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/native/external/zlib-ng/deflate_fast.c b/src/native/external/zlib-ng/deflate_fast.c
index 3184aa718c7..6c6e227136b 100644
--- a/src/native/external/zlib-ng/deflate_fast.c
+++ b/src/native/external/zlib-ng/deflate_fast.c
@@ -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);

diff --git a/src/native/external/zlib-ng/deflate_quick.c b/src/native/external/zlib-ng/deflate_quick.c
index df5a17b9e66..8dd32ae32a9 100644
--- a/src/native/external/zlib-ng/deflate_quick.c
+++ b/src/native/external/zlib-ng/deflate_quick.c
@@ -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;
diff --git a/src/native/external/zlib-ng/deflate_rle.c b/src/native/external/zlib-ng/deflate_rle.c
index cd085099460..cf1f3bf4bba 100644
--- a/src/native/external/zlib-ng/deflate_rle.c
+++ b/src/native/external/zlib-ng/deflate_rle.c
@@ -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);

diff --git a/src/native/external/zlib-ng/deflate_slow.c b/src/native/external/zlib-ng/deflate_slow.c
index 9f1c913467b..b71c0e5a331 100644
--- a/src/native/external/zlib-ng/deflate_slow.c
+++ b/src/native/external/zlib-ng/deflate_slow.c
@@ -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);

--
2.45.2.windows.1

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/

We have also applied the custom patches under the patches/zlib-ng/ folder.
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 0 additions & 2 deletions src/native/external/zlib-ng.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ 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
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand Down
4 changes: 3 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 @@ -7,6 +7,7 @@
#include "acle_intrins.h"
#include "../../zbuild.h"
#include "../../deflate.h"
#include <assert.h>

/* SIMD version of hash_chain rebase */
static inline void slide_hash_chain(Pos *table, uint32_t entries, uint16_t wsize) {
Expand Down Expand Up @@ -39,7 +40,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);
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
4 changes: 3 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 @@ -12,6 +12,7 @@
#include "neon_intrins.h"
#include "../../zbuild.h"
#include "../../deflate.h"
#include <assert.h>

/* SIMD version of hash_chain rebase */
static inline void slide_hash_chain(Pos *table, uint32_t entries, uint16_t wsize) {
Expand All @@ -38,7 +39,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);
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
4 changes: 3 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 @@ -6,6 +6,7 @@
#include <altivec.h>
#include "zbuild.h"
#include "deflate.h"
#include <assert.h>

static inline void slide_hash_chain(Pos *table, uint32_t entries, uint16_t wsize) {
const vector unsigned short vmx_wsize = vec_splats(wsize);
Expand All @@ -24,7 +25,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);
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
2 changes: 2 additions & 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 @@ -10,6 +10,7 @@

#include "../../zbuild.h"
#include "../../deflate.h"
#include <assert.h>

static inline void slide_hash_chain(Pos *table, uint32_t entries, uint16_t wsize) {
size_t vl;
Expand All @@ -25,6 +26,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);
uint16_t wsize = (uint16_t)s->w_size;

slide_hash_chain(s->head, HASH_SIZE, wsize);
Expand Down
2 changes: 2 additions & 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 @@ -13,6 +13,7 @@
#include "../../deflate.h"

#include <immintrin.h>
#include <assert.h>

static inline void slide_hash_chain(Pos *table, uint32_t entries, const __m256i wsize) {
table += entries;
Expand All @@ -31,6 +32,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);
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);
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