Re: [boost] vc_8 and 1.33.1 [regex]

From: John Maddock (john_at_[hidden])
Date: 2005-12-12 08:12:09

> I think that the problem is at regex_traits_defaults.hpp line 191:
>
> static const character_pointer_range<charT>* ranges_end = ranges +
> (sizeof(ranges)/sizeof(ranges[0]));
>
> Your algorithm doesn't allow for compiler data alignment. With 8-byte
> data alignment (now the default with VC8) sizeof(ranges) ==
> ((19*4)/8+1)*8 == 80. The compiler warning is correct 
> get_default_class_id() may return a value between -1 and 20.  Add 1
> to this return value and you get a range of 0-21 which exceeds the
> size of masks in lookup_classname_imp().

I don't believe that's correct: for one thing the trick:

#define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))

is very well known and heavily used (in the Linux kernal for one thing), I 
can't believe that MS would break such a well used idiom.  For another 
adding a static_assert in there *does* pass, and in any case each element in 
the array is 8-bytes (32-bit pointers) or 16-bytes (64-bit pointers), so the 
alignment and size must be a multiple of 8 even if there are an odd number 
of elements.

> Note that similar code at win32_regex_traits.hpp line 536 is not
> affected because the size of masks is 8-byte aligned (for now at
> least).  64-bit compilers require 8-byte alignment so I assume that
> this code will not compile correctly on those platforms as well.
>
> If you must use this construct ("sizeof(ranges)/sizeof(ranges[0]")
> then static assert that this formula yields the same size as your
> array.
> I see that there are a few other non-trivial warnings in my build of
> the regex library.  My examples were not intended to be exhaustive
> but to illustrate the utility of incorporating this into boost's QA.

Understood, but what other warnings were you seeing?

I'll ask about testing with /analyse, but I do wonder whether the compile 
times may be too long for it to be practical?

Thanks, John.