MarkReedZ / mrhttp

The fastest python web framework - 8.5m requests per second!
MIT License
19 stars 6 forks source link

AddressSanitizer false positive #9

Open MarkReedZ opened 6 years ago

MarkReedZ commented 6 years ago

Building with the address sanitizer gives I believe a false error when you access a page on the site. In the code we're using cmpestri pointing to a staticly initialized array along with a length of that array. The complaint occurs because of the 128bit load I believe when the array is smaller than that.

I assume we can get rid of this error by initializing all occurrences of static char ranges with the full length expected by the load but haven't tried it and am not sure of the performance impact if any. Also can we use an aligned load?

CFLAGS='-Wall -O0 -g -fsanitize=address -fno-omit-frame-pointer -fsanitize-recover=address' python setup.py install --force
ASAN_OPTIONS=halt_on_error=0 LD_PRELOAD=/usr/lib/gcc/x86_64-linux-gnu/7/libasan.so python tst.py

==15521==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7f18aa246b20 at pc 0x7f18aa011b7a bp 0x7ffd42ea2910 sp 0x7ffd42ea2900
READ of size 16 at 0x7f18aa246b20 thread T0
    #0 0x7f18aa011b79 in _mm_loadu_si128 /usr/lib/gcc/x86_64-linux-gnu/7/include/emmintrin.h:702
    #1 0x7f18aa011b79 in findchar_fast src/mrhttp/internals/request.c:158

// ranges that is loaded
  static char ranges1[] = "%%" "??";

// This just points to ranges1
        __m128i ranges16 = _mm_loadu_si128((const __m128i *)ranges);

// _mm_cmpestri takes a size so it doesn't got beyond the length of the array passed in.
            int r = _mm_cmpestri(ranges16, ranges_size, b16, 16, _SIDD_LEAST_SIGNIFICANT | _SIDD_CMP_RANGES | _SIDD_UBYTE_OPS);
MarkReedZ commented 5 years ago

I'm pretty sure simply setting the array length will fix this.

static char ranges1[16] = "%%" "??";