_MEM_WRAP_NEEDS_RUNTIME_CHECK (malloc wrapping) in handy.h broken #16231

p5pRT · 2017-11-08T00:31:51Z

Migrated from rt.perl.org#132415 (status was 'resolved')

Searchable as RT132415$

p5pRT · 2017-11-08T00:31:51Z

From @mauke

Created by @mauke

In handy.h there is this piece of code:

# define _MEM_WRAP_NEEDS_RUNTIME_CHECK(n,t) \
(8 * sizeof(n) + sizeof(t) > sizeof(MEM_SIZE))

A comment claims:

* It's mathematically equivalent to
* max(n) * sizeof(t) > MEM_SIZE_MAX

This is false because _MEM_WRAP_NEEDS_RUNTIME_CHECK currently always returns
true (it's broken).

It was lasted touched in commit 004073b
"Simplify _MEM_WRAP_NEEDS_RUNTIME_CHECK()".

The old version was:

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

I don't understand the new version, so I wrote a little C program to compare
the two. Here's the output (compiled on a 32-bit platform):

Perl Info


Flags:
    category=core
    severity=low

Site configuration information for perl 5.26.0:

Configured by mauke at Fri Sep 22 13:28:36 CEST 2017.

Summary of my perl5 (revision 5 version 26 subversion 0) configuration:
   
  Platform:
    osname=linux
    osvers=4.9.41-1-lts
    archname=i686-linux
    uname='linux simplicio 4.9.41-1-lts #1 smp mon aug 7 17:57:02 cest 2017 i686 gnulinux '
    config_args=''
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=undef
    usemultiplicity=undef
    use64bitint=undef
    use64bitall=undef
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
    bincompat5005=undef
  Compiler:
    cc='cc'
    ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
    optimize='-O2 -march=native'
    cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='7.2.0'
    gccosandvers=''
    intsize=4
    longsize=4
    ptrsize=4
    doublesize=8
    byteorder=1234
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=12
    longdblkind=3
    ivtype='long'
    ivsize=4
    nvtype='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=4
    prototype=define
  Linker and Libraries:
    ld='cc'
    ldflags ='-fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/gcc/i686-pc-linux-gnu/7.2.0/include-fixed /usr/lib /lib
    libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.26.so
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.26'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -O2 -march=native -L/usr/local/lib -fstack-protector-strong'



@INC for perl 5.26.0:
    /home/mauke/usr/lib/perl5/site_perl/5.26.0/i686-linux
    /home/mauke/usr/lib/perl5/site_perl/5.26.0
    /home/mauke/usr/lib/perl5/5.26.0/i686-linux
    /home/mauke/usr/lib/perl5/5.26.0


Environment for perl 5.26.0:
    HOME=/home/mauke
    LANG=en_US.UTF-8
    LANGUAGE=en_US
    LC_COLLATE=C
    LC_MONETARY=de_DE.UTF-8
    LC_TIME=de_DE.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/mauke/perl5/perlbrew/bin:/home/mauke/bin:/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl
    PERLBREW_BASHRC_VERSION=0.73
    PERLBREW_HOME=/home/mauke/.perlbrew
    PERLBREW_ROOT=/home/mauke/perl5/perlbrew
    PERL_BADLANG (unset)
    PERL_UNICODE=SAL
    SHELL=/bin/bash

p5pRT · 2017-11-08T00:33:19Z

From @mauke

On Tue, 07 Nov 2017 16:31:51 -0800, mauke- wrote:

I don't understand the new version, so I wrote a little C program to
compare
the two. Here's the output (compiled on a 32-bit platform):

( uchar, char ) max(1) * 1 > UINT4_MAX? | old:no |
new:yes | expected:no (!)
(ushort, char ) max(2) * 1 > UINT4_MAX? | old:no |
new:yes | expected:no (!)
( uint, char ) max(4) * 1 > UINT4_MAX? | old:no |
new:yes | expected:no (!)
( ulong, char ) max(4) * 1 > UINT4_MAX? | old:no |
new:yes | expected:no (!)
( ulong, char ) max(4) * 1 > UINT4_MAX? | old:no |
new:yes | expected:no (!)
(ullong, char ) max(8) * 1 > UINT4_MAX? | old:yes |
new:yes | expected:yes
( uchar, char [2] ) max(1) * 2 > UINT4_MAX? | old:no |
new:yes | expected:no (!)
(ushort, char [2] ) max(2) * 2 > UINT4_MAX? | old:no |
new:yes | expected:no (!)
( uint, char [2] ) max(4) * 2 > UINT4_MAX? | old:yes |
new:yes | expected:yes
( ulong, char [2] ) max(4) * 2 > UINT4_MAX? | old:yes |
new:yes | expected:yes
( ulong, char [2] ) max(4) * 2 > UINT4_MAX? | old:yes |
new:yes | expected:yes
(ullong, char [2] ) max(8) * 2 > UINT4_MAX? | old:yes |
new:yes | expected:yes
( uchar, char [0xffff] ) max(1) * 65535 > UINT4_MAX? | old:no |
new:yes | expected:no (!)
(ushort, char [0xffff] ) max(2) * 65535 > UINT4_MAX? | old:no |
new:yes | expected:no (!)
( uint, char [0xffff] ) max(4) * 65535 > UINT4_MAX? | old:yes |
new:yes | expected:yes
( ulong, char [0xffff] ) max(4) * 65535 > UINT4_MAX? | old:yes |
new:yes | expected:yes
( ulong, char [0xffff] ) max(4) * 65535 > UINT4_MAX? | old:yes |
new:yes | expected:yes
(ullong, char [0xffff] ) max(8) * 65535 > UINT4_MAX? | old:yes |
new:yes | expected:yes
( uchar, char [0xffff+1]) max(1) * 65536 > UINT4_MAX? | old:no |
new:yes | expected:no (!)
(ushort, char [0xffff+1]) max(2) * 65536 > UINT4_MAX? | old:no |
new:yes | expected:no (!)
( uint, char [0xffff+1]) max(4) * 65536 > UINT4_MAX? | old:yes |
new:yes | expected:yes
( ulong, char [0xffff+1]) max(4) * 65536 > UINT4_MAX? | old:yes |
new:yes | expected:yes
( ulong, char [0xffff+1]) max(4) * 65536 > UINT4_MAX? | old:yes |
new:yes | expected:yes
(ullong, char [0xffff+1]) max(8) * 65536 > UINT4_MAX? | old:yes |
new:yes | expected:yes

Code attached.

p5pRT · 2017-11-08T00:33:19Z

From @mauke

#include <stdio.h>
#include <limits.h>
#include <stdint.h>

#ifndef MEM_SIZE
#define MEM_SIZE size_t
#endif
#define MEM_SIZE_MAX ((MEM_SIZE)-1)

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

#define MEM_WRAP_NEEDS_RUNTIME_CHECK_2(n, t) \
    (8 * sizeof(n) + sizeof(t) > sizeof(MEM_SIZE))

#define ACTUALLY_OVERFLOWS(n, t) \
    ((n) > MEM_SIZE_MAX / sizeof(t))

#define TRY_WITH2(n, t) \
    printf("(%6s, %-15s) max(%zu) * %5zu > UINT%zu_MAX? | old:%-3s | new:%-3s | expected:%-3s %s\n", \
        #n, #t, \
        sizeof (n), sizeof (t), \
        sizeof (MEM_SIZE), \
        MEM_WRAP_NEEDS_RUNTIME_CHECK_1(n, t) ? "yes" : "no", \
        MEM_WRAP_NEEDS_RUNTIME_CHECK_2(n, t) ? "yes" : "no", \
        ACTUALLY_OVERFLOWS(n, t) ? "yes" : "no", \
        MEM_WRAP_NEEDS_RUNTIME_CHECK_1(n, t) != ACTUALLY_OVERFLOWS(n, t) || \
        MEM_WRAP_NEEDS_RUNTIME_CHECK_2(n, t) != ACTUALLY_OVERFLOWS(n, t) ? "(!)" : "")

#define TRY_WITH(t) \
    do { \
        const unsigned char uchar = UCHAR_MAX; \
        const unsigned short ushort = USHRT_MAX; \
        const unsigned int uint = UINT_MAX; \
        const unsigned long ulong = ULONG_MAX; \
        const unsigned long long ullong = ULLONG_MAX; \
        TRY_WITH2(uchar,   t); \
        TRY_WITH2(ushort,  t); \
        TRY_WITH2(uint,    t); \
        TRY_WITH2(ulong,   t); \
        TRY_WITH2(ulong,   t); \
        TRY_WITH2(ullong,  t); \
    } while (0)

int main(void) {
    TRY_WITH(char);

    TRY_WITH(char [2]);

    TRY_WITH(char [0xffff]);
    TRY_WITH(char [0xffff+1]);
}

p5pRT · 2017-11-11T12:57:27Z

From @mauke

On Tue, 07 Nov 2017 16:31:51 -0800, mauke- wrote:

In handy.h there is this piece of code:

# define _MEM_WRAP_NEEDS_RUNTIME_CHECK(n,t) \
(8 * sizeof(n) + sizeof(t) > sizeof(MEM_SIZE))

A comment claims:

* It's mathematically equivalent to
* max(n) * sizeof(t) > MEM_SIZE_MAX

This is false because _MEM_WRAP_NEEDS_RUNTIME_CHECK currently always
returns
true (it's broken).

It was lasted touched in commit
004073b
"Simplify _MEM_WRAP_NEEDS_RUNTIME_CHECK()".

The old version was:

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

I have now reverted this commit in 445198b.

I believe the new version could have been fixed by changing

(8 * sizeof(n) + sizeof(t) > sizeof(MEM_SIZE))

to

(8 * sizeof(n) + ILOG2(sizeof(t)) > 8 * sizeof(MEM_SIZE))

but then I'd have to define a macro to compute the integer log base 2 (rounded up) of sizeof(t) at compile time. This is possible but annoying (64 separate comparisons). It's also the opposite of a simplification.

So unless we really need to do that, I'd rather not.

p5pRT · 2018-03-06T13:08:40Z

From @iabyn

On Sat, Nov 11, 2017 at 04:57:27AM -0800, l.mai@web.de via RT wrote:

On Tue, 07 Nov 2017 16:31:51 -0800, mauke- wrote:

In handy.h there is this piece of code:

# define _MEM_WRAP_NEEDS_RUNTIME_CHECK(n,t) \
(8 * sizeof(n) + sizeof(t) > sizeof(MEM_SIZE))

A comment claims:

* It's mathematically equivalent to
* max(n) * sizeof(t) > MEM_SIZE_MAX

This is false because _MEM_WRAP_NEEDS_RUNTIME_CHECK currently always
returns
true (it's broken).

It was lasted touched in commit
004073b
"Simplify _MEM_WRAP_NEEDS_RUNTIME_CHECK()".

The old version was:

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

I have now reverted this commit in 445198b.

I believe the new version could have been fixed by changing
$8 \* sizeof\(n$ \+ sizeof$t$ > sizeof$MEM\_SIZE$\)
to
$8 \* sizeof\(n$ \+ ILOG2$sizeof\(t$\) > 8 \* sizeof$MEM\_SIZE$\)
but then I'd have to define a macro to compute the integer log base 2 (rounded up) of sizeof(t) at compile time. This is possible but annoying (64 separate comparisons). It's also the opposite of a simplification.

So unless we really need to do that, I'd rather not.

I belatedly agree with your analysis. I clearly had a brain fart that
day. I'll close the ticket.

--
A walk of a thousand miles begins with a single step...
then continues for another 1,999,999 or so.

p5pRT · 2018-03-06T13:08:40Z

The RT System itself - Status changed from 'new' to 'open'

p5pRT · 2018-03-06T15:22:47Z

From @khwilliamson

Offlist

On 03/06/2018 06:08 AM, Dave Mitchell wrote:

I clearly had a brain fart that
day.

I'm glad to find out that happens to younger people than me, too.

p5pRT · 2018-03-06T15:24:35Z

From @demerphq

On 6 March 2018 at 16:21, Karl Williamson <public@khwilliamson.com> wrote:

Offlist

On 03/06/2018 06:08 AM, Dave Mitchell wrote:

I clearly had a brain fart that
day.

I'm glad to find out that happens to younger people than me, too.

Happens to the younger folks more probably, shorter attention sp

Ooh! Look! A butterfly!

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

p5pRT · 2018-03-06T20:47:41Z

@iabyn - Status changed from 'open' to 'resolved'

p5pRT closed this as completed Mar 6, 2018

p5pRT added Severity Low type-core labels Oct 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

_MEM_WRAP_NEEDS_RUNTIME_CHECK (malloc wrapping) in handy.h broken #16231

_MEM_WRAP_NEEDS_RUNTIME_CHECK (malloc wrapping) in handy.h broken #16231

p5pRT commented Nov 8, 2017

p5pRT commented Nov 8, 2017

p5pRT commented Nov 8, 2017

p5pRT commented Nov 8, 2017

p5pRT commented Nov 11, 2017

p5pRT commented Mar 6, 2018

p5pRT commented Mar 6, 2018

p5pRT commented Mar 6, 2018

p5pRT commented Mar 6, 2018

p5pRT commented Mar 6, 2018

_MEM_WRAP_NEEDS_RUNTIME_CHECK (malloc wrapping) in handy.h broken #16231

_MEM_WRAP_NEEDS_RUNTIME_CHECK (malloc wrapping) in handy.h broken #16231

Comments

p5pRT commented Nov 8, 2017

p5pRT commented Nov 8, 2017

From @mauke

Created by @mauke

p5pRT commented Nov 8, 2017

From @mauke

p5pRT commented Nov 8, 2017

From @mauke

p5pRT commented Nov 11, 2017

From @mauke

p5pRT commented Mar 6, 2018

From @iabyn

p5pRT commented Mar 6, 2018

p5pRT commented Mar 6, 2018

From @khwilliamson

p5pRT commented Mar 6, 2018

From @demerphq

p5pRT commented Mar 6, 2018