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.

Sign up for GitHub

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

Address most implicit 64-to-32-bit conversion warnings. #525

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Address most implicit 64-to-32-bit conversion warnings. #525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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/pcre2.h.generic
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ PCRE2_EXP_DECL int PCRE2_CALL_CONVENTION \
PCRE2_EXP_DECL int PCRE2_CALL_CONVENTION \
pcre2_set_depth_limit(pcre2_match_context *, uint32_t); \
PCRE2_EXP_DECL int PCRE2_CALL_CONVENTION \
pcre2_set_heap_limit(pcre2_match_context *, uint32_t); \
pcre2_set_heap_limit(pcre2_match_context *, PCRE2_SIZE); \
Copy link
Collaborator

Choose a reason for hiding this comment

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

API functions cannot be changed, even if they are not the best.

Copy link
Author

Choose a reason for hiding this comment

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

Oops, yeah, I didn't mean to introduce an ABI break. :-/

Copy link
Author

Choose a reason for hiding this comment

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

Ah, right, I'll need to revert the POSIX API changes too.

PCRE2_EXP_DECL int PCRE2_CALL_CONVENTION \
pcre2_set_match_limit(pcre2_match_context *, uint32_t); \
PCRE2_EXP_DECL int PCRE2_CALL_CONVENTION \
Expand Down
2 changes: 1 addition & 1 deletion src/pcre2.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ PCRE2_EXP_DECL int PCRE2_CALL_CONVENTION \
PCRE2_EXP_DECL int PCRE2_CALL_CONVENTION \
pcre2_set_depth_limit(pcre2_match_context *, uint32_t); \
PCRE2_EXP_DECL int PCRE2_CALL_CONVENTION \
pcre2_set_heap_limit(pcre2_match_context *, uint32_t); \
pcre2_set_heap_limit(pcre2_match_context *, PCRE2_SIZE); \
PCRE2_EXP_DECL int PCRE2_CALL_CONVENTION \
pcre2_set_match_limit(pcre2_match_context *, uint32_t); \
PCRE2_EXP_DECL int PCRE2_CALL_CONVENTION \
Expand Down
2 changes: 1 addition & 1 deletion src/pcre2_chkdint.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ The INT64_OR_DOUBLE type is a 64-bit integer type when available,
otherwise double. */

BOOL
PRIV(ckd_smul)(PCRE2_SIZE *r, int a, int b)
PRIV(ckd_smul)(PCRE2_SIZE *r, ptrdiff_t a, ptrdiff_t b)
Copy link
Contributor

Choose a reason for hiding this comment

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

the whole point of this function is to make sure those 2 parameters are intso we can check for overflow

Copy link
Author

Choose a reason for hiding this comment

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

Ah, yeah, the hand-coded branch would need reworking. (__builtin_mul_overflow should DTRT regardless.) However, without this change, pcre2test's process_data could in principle pass in a truncated replen, yielding incorrect results. Should it instead bail if replen > INT_MAX?

Copy link
Contributor

@carenas carenas Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

pcre2test is a test application, AFAIK replen was made a size_t just to avoid having to do a cast, and it is expected never to have a value larger than int either, so a truncation there shouldn't be possible, but feel free to poke at it and prove me wrong.

the call for ckd_smul was indeed how we are making sure that the value of the multiplication is always correct.

FWIW you could NOT build this test application if all you want is the library an want it to be warning free, there is a configure and cmake option for that. You haven't answered my original question so I am still not sure what the end goal is.

{
#ifdef HAVE_BUILTIN_MUL_OVERFLOW
PCRE2_SIZE m;
Expand Down
2 changes: 1 addition & 1 deletion src/pcre2_context.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ return 0;
}

PCRE2_EXP_DEFN int PCRE2_CALL_CONVENTION
pcre2_set_heap_limit(pcre2_match_context *mcontext, uint32_t limit)
pcre2_set_heap_limit(pcre2_match_context *mcontext, PCRE2_SIZE limit)
{
mcontext->heap_limit = limit;
return 0;
Expand Down
2 changes: 1 addition & 1 deletion src/pcre2_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2116,7 +2116,7 @@ extern void * _pcre2_memmove(void *, const void *, size_t);

#endif /* PCRE2_CODE_UNIT_WIDTH */

extern BOOL PRIV(ckd_smul)(PCRE2_SIZE *, int, int);
extern BOOL PRIV(ckd_smul)(PCRE2_SIZE *, ptrdiff_t, ptrdiff_t);

#include "pcre2_util.h"

Expand Down
6 changes: 3 additions & 3 deletions src/pcre2_intmodedep.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ typedef struct pcre2_real_match_context {
uint32_t (*substitute_case_callout)(uint32_t, int, void *);
void *substitute_case_callout_data;
PCRE2_SIZE offset_limit;
uint32_t heap_limit;
PCRE2_SIZE heap_limit;
uint32_t match_limit;
uint32_t depth_limit;
} pcre2_real_match_context;
Expand Down Expand Up @@ -889,7 +889,7 @@ doing traditional NFA matching (pcre2_match() and friends). */

typedef struct match_block {
pcre2_memctl memctl; /* For general use */
uint32_t heap_limit; /* As it says */
PCRE2_SIZE heap_limit; /* As it says */
uint32_t match_limit; /* As it says */
uint32_t match_limit_depth; /* As it says */
uint32_t match_call_count; /* Number of times a new frame is created */
Expand Down Expand Up @@ -943,7 +943,7 @@ typedef struct dfa_match_block {
PCRE2_SPTR last_used_ptr; /* Latest consulted character */
const uint8_t *tables; /* Character tables */
PCRE2_SIZE start_offset; /* The start offset value */
uint32_t heap_limit; /* As it says */
PCRE2_SIZE heap_limit; /* As it says */
PCRE2_SIZE heap_used; /* As it says */
uint32_t match_limit; /* As it says */
uint32_t match_limit_depth; /* As it says */
Expand Down
9 changes: 5 additions & 4 deletions src/pcre2_jit_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2362,7 +2362,7 @@ SLJIT_ASSERT(stackpos == STACK(stacktop));
typedef struct delayed_mem_copy_status {
struct sljit_compiler *compiler;
int store_bases[RECURSE_TMP_REG_COUNT];
int store_offsets[RECURSE_TMP_REG_COUNT];
sljit_sw store_offsets[RECURSE_TMP_REG_COUNT];
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are intentionally small values.

Copy link
Author

Choose a reason for hiding this comment

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

Without this change, delayed_mem_copy_move's assignment into store_offsets can wind up truncated. Attempting to narrow the type of its store_offset argument instead reveals that three call sites pass in 64-bit values.

int tmp_regs[RECURSE_TMP_REG_COUNT];
int saved_tmp_regs[RECURSE_TMP_REG_COUNT];
int next_tmp_reg;
Expand Down Expand Up @@ -3302,7 +3302,8 @@ OP2(SLJIT_SUB | SLJIT_SET_Z, COUNT_MATCH, 0, COUNT_MATCH, 0, SLJIT_IMM, 1);
add_jump(compiler, &common->calllimit, JUMP(SLJIT_ZERO));
}

static SLJIT_INLINE void allocate_stack(compiler_common *common, int size)
static SLJIT_INLINE void allocate_stack(compiler_common *common,
PCRE2_SIZE size)
{
/* May destroy all locals and registers except TMP2. */
DEFINE_COMPILER;
Expand All @@ -3319,7 +3320,7 @@ OP1(SLJIT_MOV, SLJIT_MEM1(SLJIT_SP), LOCALS1, TMP1, 0);
add_stub(common, CMP(SLJIT_LESS, STACK_TOP, 0, STACK_LIMIT, 0));
}

static SLJIT_INLINE void free_stack(compiler_common *common, int size)
static SLJIT_INLINE void free_stack(compiler_common *common, PCRE2_SIZE size)
{
DEFINE_COMPILER;

Expand Down Expand Up @@ -4064,7 +4065,7 @@ if (common->invalid_utf)
#define READ_CHAR_NEWLINE (READ_CHAR_UPDATE_STR_PTR | READ_CHAR_UTF8_NEWLINE)
#define READ_CHAR_VALID_UTF 0x4

static void read_char(compiler_common *common, sljit_u32 min, sljit_u32 max,
static void read_char(compiler_common *common, sljit_uw min, sljit_uw max,
jump_list **backtracks, sljit_u32 options)
{
/* Reads the precise value of a character into TMP1, if the character is
Expand Down
4 changes: 2 additions & 2 deletions src/pcre2_jit_neon_inc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ POSSIBILITY OF SUCH DAMAGE.
&& ((__clang_major__ == 3 && __clang_minor__ >= 3) || (__clang_major__ > 3)))
__attribute__((no_sanitize_address))
#endif
static sljit_u8* SLJIT_FUNC FF_FUN(sljit_u8 *str_end, sljit_u8 **str_ptr, sljit_uw offs1, sljit_uw offs2, sljit_uw chars)
static sljit_u8* SLJIT_FUNC FF_FUN(sljit_u8 *str_end, sljit_u8 **str_ptr, sljit_uw offs1, sljit_uw offs2, sljit_u32 chars)
Copy link
Contributor

Choose a reason for hiding this comment

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

this should break the 16bit library

Copy link
Author

@ucko ucko Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Not AFAICT, though I must admit I'd initially focused on the 8-bit one.

#undef FF_FUN
{
quad_word qw;
Expand Down Expand Up @@ -119,7 +119,7 @@ vect_t vmask = VDUPQ(mask);
compare_type compare1_type = compare_match1;
compare_type compare2_type = compare_match1;
vect_t cmp1a, cmp1b, cmp2a, cmp2b;
const sljit_u32 diff = IN_UCHARS(offs1 - offs2);
const sljit_uw diff = IN_UCHARS(offs1 - offs2);
PCRE2_UCHAR char1a = ic.c.c1;
PCRE2_UCHAR char2a = ic.c.c3;

Expand Down
26 changes: 14 additions & 12 deletions src/pcre2grep.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,7 @@ return isatty(fileno(f));
/************* Print optionally coloured match Unix-style and z/OS **********/

static void
print_match(const void *buf, int length)
print_match(const void *buf, size_t length)
{
if (length == 0) return;
if (do_colour) fprintf(stdout, "%c[%sm", 0x1b, colour_string);
Expand Down Expand Up @@ -1301,7 +1301,7 @@ Returns: TRUE if the path is not excluded
static BOOL
test_incexc(char *path, patstr *ip, patstr *ep)
{
int plen = strlen((const char *)path);
size_t plen = strlen((const char *)path);

for (; ep != NULL; ep = ep->next)
{
Expand Down Expand Up @@ -2590,15 +2590,17 @@ fill_buffer(void *handle, int frtype, char *buffer, PCRE2_SIZE length,
(void)frtype; /* Avoid warning when not used */

#ifdef SUPPORT_LIBZ
if (frtype == FR_LIBZ)
return gzread((gzFile)handle, buffer, length);
else
if (frtype == FR_LIBZ) {
if (length > UINT_MAX) length = UINT_MAX;
return gzread((gzFile)handle, buffer, (unsigned int) length);
} else
#endif

#ifdef SUPPORT_LIBBZ2
if (frtype == FR_LIBBZ2)
return (PCRE2_SIZE)BZ2_bzread((BZFILE *)handle, buffer, length);
else
if (frtype == FR_LIBBZ2) {
if (length > INT_MAX) length = INT_MAX;
return (PCRE2_SIZE)BZ2_bzread((BZFILE *)handle, buffer, (int) length);
} else
#endif

return (input_line_buffered ?
Expand Down Expand Up @@ -2910,7 +2912,7 @@ while (ptr < endptr)
int n = om->groupnum;
if (n == 0 || n < mrc)
{
int plen = offsets[2*n + 1] - offsets[2*n];
size_t plen = offsets[2*n + 1] - offsets[2*n];
if (plen > 0)
{
if (printed && om_separator != NULL)
Expand Down Expand Up @@ -3414,7 +3416,7 @@ if (isdirectory(pathname))
while ((nextfile = readdirectory(dir)) != NULL)
{
int frc;
int fnlength = strlen(pathname) + strlen(nextfile) + 2;
size_t fnlength = strlen(pathname) + strlen(nextfile) + 2;
if (fnlength > FNBUFSIZ)
{
/* LCOV_EXCL_START - this is a "never" event */
Expand Down Expand Up @@ -4231,9 +4233,9 @@ for (i = 1; i < argc; i++)
else
{
unsigned long int n = decode_number(option_data, op, longop);
if (op->type == OP_U32NUMBER) *((uint32_t *)op->dataptr) = n;
if (op->type == OP_U32NUMBER) *((uint32_t *)op->dataptr) = (uint32_t) n;
else if (op->type == OP_SIZE) *((PCRE2_SIZE *)op->dataptr) = n;
else *((int *)op->dataptr) = n;
else *((int *)op->dataptr) = (int) n;
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/pcre2posix.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,8 @@ PCRE2POSIX_EXP_DEFN int PCRE2_CALL_CONVENTION
pcre2_regexec(const regex_t *preg, const char *string, size_t nmatch,
regmatch_t pmatch[], int eflags)
{
int rc, so, eo;
int rc;
regoff_t so, eo;
int options = 0;
pcre2_match_data *md = (pcre2_match_data *)preg->re_match_data;

Expand Down
5 changes: 3 additions & 2 deletions src/pcre2posix.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ POSSIBILITY OF SUCH DAMAGE.
#ifndef PCRE2POSIX_H_IDEMPOTENT_GUARD
#define PCRE2POSIX_H_IDEMPOTENT_GUARD

/* Have to include stdlib.h in order to ensure that size_t is defined. */
/* Respectively ensure that ptrdiff_t and size_t are defined. */

#include <stddef.h>
#include <stdlib.h>

/* Allow for C++ users */
Expand Down Expand Up @@ -111,7 +112,7 @@ typedef struct {

/* The structure in which a captured offset is returned. */

typedef int regoff_t;
typedef ptrdiff_t regoff_t;

typedef struct {
regoff_t rm_so;
Expand Down
4 changes: 2 additions & 2 deletions src/pcre2posix_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,11 @@ for (int i = 0; i < count; i++)
fprintf(stderr, "Mismatched results for successful match\n");
fprintf(stderr, "Pattern is: %s\n", pattern);
fprintf(stderr, "Subject is: %s\n", *subjects);
fprintf(stderr, "Result %d: expected %d %d received %d %d\n",
fprintf(stderr, "Result %d: expected %d %d received %zd %zd\n",
j, rd[-1], rd[0], m->rm_so, m->rm_eo);
return 1;
}
PRINTF(" (%d %d %d)", j, m->rm_so, m->rm_eo);
PRINTF(" (%d %zd %zd)", j, m->rm_so, m->rm_eo);
}
}

Expand Down
Loading
Loading