Skip to content

Commit

Permalink
fix some 32/64-bit compiler warnings
Browse files Browse the repository at this point in the history 
Some bits of code don't do well on a 32-bit system with 64-bit ints
(-Duse64bitint)

In particular:

_MEM_WRAP_NEEDS_RUNTIME_CHECK:
    if sizeof(MEM_SIZE) > sizeof(n), then the shift count could be
        negative

S_regmatch:
    ln and n were two different sizes and signesses, so comparing them
    warned. Since they were being mis-used as two convenient temporary
    booleans anyway, just use temporary booleans instead.

Perl_sv_vcatpvfn_flags:
    the test/assertion (IV)elen < 0 was (I think) being used to test for
    signed/unsigned conversion wrap-around. elen is of type STRLEN which
    is a pointer-based type, so can be 32-bit while IV is 64-bit. Instead
    compare it to half the maximum value of a STRLEN var to see if it may
    have wrapped.
  • Loading branch information
@iabyn
iabyn committed Sep 24, 2015
1 parent e120c24 commit f25d48a
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 23 deletions.
5 changes: 4 additions & 1 deletion handy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1917,10 +1917,13 @@ PoisonWith(0xEF) for catching access to freed memory.
* As well as avoiding the need for a run-time check in some cases, it's
* designed to avoid compiler warnings like:
* comparison is always false due to limited range of data type
* It's mathematically equivalent to
* max(n) * sizeof(t) > MEM_SIZE_MAX
*/

# define _MEM_WRAP_NEEDS_RUNTIME_CHECK(n,t) \
(sizeof(t) > ((MEM_SIZE)1 << 8*(sizeof(MEM_SIZE) - sizeof(n))))
( sizeof(MEM_SIZE) <= sizeof(n) \
|| sizeof(t) > ((MEM_SIZE)1 << 8*(sizeof(MEM_SIZE) - sizeof(n))))

/* This is written in a slightly odd way to avoid various spurious
* compiler warnings. We *want* to write the expression as
Expand Down
48 changes: 30 additions & 18 deletions regexec.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1205,10 +1205,10 @@ Perl_re_intuit_start(pTHX_
* didn't contradict, so just retry the anchored "other"
* substr */
DEBUG_EXECUTE_r(PerlIO_printf(Perl_debug_log,
" Found /%s^%s/m, rescanning for anchored from offset %ld (rx_origin now %"IVdf")...\n",
" Found /%s^%s/m, rescanning for anchored from offset %"IVdf" (rx_origin now %"IVdf")...\n",
PL_colors[0], PL_colors[1],
(long)(rx_origin - strbeg + prog->anchored_offset),
(long)(rx_origin - strbeg)
(IV)(rx_origin - strbeg + prog->anchored_offset),
(IV)(rx_origin - strbeg)
));
goto do_other_substr;
}
Expand Down Expand Up @@ -5526,6 +5526,8 @@ S_regmatch(pTHX_ regmatch_info *reginfo, char *startpos, regnode *prog)
/* FALLTHROUGH */

case BOUNDL: /* /\b/l */
{
bool b1, b2;
_CHECK_AND_WARN_PROBLEMATIC_LOCALE;

if (FLAGS(scan) != TRADITIONAL_BOUND) {
Expand All @@ -5538,27 +5540,28 @@ S_regmatch(pTHX_ regmatch_info *reginfo, char *startpos, regnode *prog)

if (utf8_target) {
if (locinput == reginfo->strbeg)
ln = isWORDCHAR_LC('\n');
b1 = isWORDCHAR_LC('\n');
else {
ln = isWORDCHAR_LC_utf8(reghop3((U8*)locinput, -1,
b1 = isWORDCHAR_LC_utf8(reghop3((U8*)locinput, -1,
(U8*)(reginfo->strbeg)));
}
n = (NEXTCHR_IS_EOS)
b2 = (NEXTCHR_IS_EOS)
? isWORDCHAR_LC('\n')
: isWORDCHAR_LC_utf8((U8*)locinput);
}
else { /* Here the string isn't utf8 */
ln = (locinput == reginfo->strbeg)
b1 = (locinput == reginfo->strbeg)
? isWORDCHAR_LC('\n')
: isWORDCHAR_LC(UCHARAT(locinput - 1));
n = (NEXTCHR_IS_EOS)
b2 = (NEXTCHR_IS_EOS)
? isWORDCHAR_LC('\n')
: isWORDCHAR_LC(nextchr);
}
if (to_complement ^ (ln == n)) {
if (to_complement ^ (b1 == b2)) {
sayNO;
}
break;
}

case NBOUND: /* /\B/ */
to_complement = 1;
Expand All @@ -5575,6 +5578,8 @@ S_regmatch(pTHX_ regmatch_info *reginfo, char *startpos, regnode *prog)
/* FALLTHROUGH */

case BOUNDA: /* /\b/a */
{
bool b1, b2;

bound_ascii_match_only:
/* Here the string isn't utf8, or is utf8 and only ascii characters
Expand All @@ -5586,16 +5591,17 @@ S_regmatch(pTHX_ regmatch_info *reginfo, char *startpos, regnode *prog)
* 2) it is a multi-byte character, in which case the final byte is
* never mistakable for ASCII, and so the test will say it is
* not a word character, which is the correct answer. */
ln = (locinput == reginfo->strbeg)
b1 = (locinput == reginfo->strbeg)
? isWORDCHAR_A('\n')
: isWORDCHAR_A(UCHARAT(locinput - 1));
n = (NEXTCHR_IS_EOS)
b2 = (NEXTCHR_IS_EOS)
? isWORDCHAR_A('\n')
: isWORDCHAR_A(nextchr);
if (to_complement ^ (ln == n)) {
if (to_complement ^ (b1 == b2)) {
sayNO;
}
break;
}

case NBOUNDU: /* /\B/u */
to_complement = 1;
Expand All @@ -5609,15 +5615,18 @@ S_regmatch(pTHX_ regmatch_info *reginfo, char *startpos, regnode *prog)
bound_utf8:
switch((bound_type) FLAGS(scan)) {
case TRADITIONAL_BOUND:
ln = (locinput == reginfo->strbeg)
{
bool b1, b2;
b1 = (locinput == reginfo->strbeg)
? 0 /* isWORDCHAR_L1('\n') */
: isWORDCHAR_utf8(reghop3((U8*)locinput, -1,
(U8*)(reginfo->strbeg)));
n = (NEXTCHR_IS_EOS)
b2 = (NEXTCHR_IS_EOS)
? 0 /* isWORDCHAR_L1('\n') */
: isWORDCHAR_utf8((U8*)locinput);
match = cBOOL(ln != n);
match = cBOOL(b1 != b2);
break;
}
case GCB_BOUND:
if (locinput == reginfo->strbeg || NEXTCHR_IS_EOS) {
match = TRUE; /* GCB always matches at begin and
Expand Down Expand Up @@ -5679,14 +5688,17 @@ S_regmatch(pTHX_ regmatch_info *reginfo, char *startpos, regnode *prog)
else { /* Not utf8 target */
switch((bound_type) FLAGS(scan)) {
case TRADITIONAL_BOUND:
ln = (locinput == reginfo->strbeg)
{
bool b1, b2;
b1 = (locinput == reginfo->strbeg)
? 0 /* isWORDCHAR_L1('\n') */
: isWORDCHAR_L1(UCHARAT(locinput - 1));
n = (NEXTCHR_IS_EOS)
b2 = (NEXTCHR_IS_EOS)
? 0 /* isWORDCHAR_L1('\n') */
: isWORDCHAR_L1(nextchr);
match = cBOOL(ln != n);
match = cBOOL(b1 != b2);
break;
}

case GCB_BOUND:
if (locinput == reginfo->strbeg || NEXTCHR_IS_EOS) {
Expand Down
10 changes: 6 additions & 4 deletions sv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11444,9 +11444,10 @@ Perl_sv_vcatpvfn_flags(pTHX_ SV *const sv, const char *const pat, const STRLEN p
is safe. */
is_utf8 = (bool)va_arg(*args, int);
elen = va_arg(*args, UV);
if ((IV)elen < 0) {
/* check if utf8 length is larger than 0 when cast to IV */
assert( (IV)elen >= 0 ); /* in DEBUGGING build we want to crash */
/* if utf8 length is larger than 0x7ffff..., then it might
* have been a signed value that wrapped */
if (elen > ((~(STRLEN)0) >> 1)) {
assert(0); /* in DEBUGGING build we want to crash */
elen= 0; /* otherwise we want to treat this as an empty string */
}
eptr = va_arg(*args, char *);
Expand Down Expand Up @@ -12690,7 +12691,8 @@ Perl_sv_vcatpvfn_flags(pTHX_ SV *const sv, const char *const pat, const STRLEN p
}
}

assert((IV)elen >= 0); /* here zero elen is fine */
/* signed value that's wrapped? */
assert(elen <= ((~(STRLEN)0) >> 1));
have = esignlen + zeros + elen;
if (have < zeros)
croak_memory_wrap();
Expand Down

0 comments on commit f25d48a

Please sign in to comment.