vector lightweight debug mode

[François Dumont]

On 12/10/2015 21:42, François Dumont wrote:
> On 07/10/2015 22:09, Jonathan Wakely wrote:
>> On 07/10/15 21:38 +0200, François Dumont wrote:
>>> Hi
>>>
>>>    I completed vector assertion mode. Here is the result of the new
>>> test you will find in the attached patch.
>>>
>>> With debug mode:
>>> /home/fdt/dev/gcc/build_git/x86_64-unknown-linux-gnu/libstdc++-v3/include/debug/safe_iterator.h:375:
>>>
>>> Error: attempt to advance a dereferenceable (start-of-sequence)
>>> iterator 2
>>> steps, which falls outside its valid range.
>>>
>>> Objects involved in the operation:
>>>    iterator @ 0x0x7fff1c346760 {
>>>      type =
>>> __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*,
>>> std::__cxx1998::vector<int, std::allocator<int> > >,
>>> std::__debug::vector<int, std::allocator<int> > > (mutable iterator);
>>>      state = dereferenceable (start-of-sequence);
>>>      references sequence with type 'std::__debug::vector<int,
>>> std::allocator<int> >' @ 0x0x7fff1c3469a0
>>>    }
>>> XFAIL: 23_containers/vector/debug/insert8_neg.cc execution test
>>>
>>>
>>> With assertion mode:
>>> /home/fdt/dev/gcc/build_git/x86_64-unknown-linux-gnu/libstdc++-v3/include/bits/stl_vector.h:1124:
>>>
>>> Error: invalid insert position outside container [begin, end) range.
>>>
>>> Objects involved in the operation:
>>>    sequence "this" @ 0x0x7fff60b1f870 {
>>>      type = std::vector<int, std::allocator<int> >;
>>>    }
>>>    iterator "__position" @ 0x0x7fff60b1f860 {
>>>      type = __gnu_cxx::__normal_iterator<int const*, std::vector<int,
>>> std::allocator<int> > >;
>>>    }
>>> XFAIL: 23_containers/vector/debug/insert8_neg.cc execution test
>> I still don't like the formatted output for the lightweight mode, it
>> adds a dependency on I/O support in libc, which is a problem for
>> embedded systems.
> I thought you just meant I/O dependency in terms of included headers.
> The __glibcxx_assert also has some I/O as in case of failure it calls:
>
>   inline void
>   __replacement_assert(const char* __file, int __line,
>                const char* __function, const char* __condition)
>   {
>     __builtin_printf("%s:%d: %s: Assertion '%s' failed.\n", __file, __line,
>              __function, __condition);
>     __builtin_abort();
>   }
>
> but it is much more limited than the _GLIBCXX_DEBUG_VERIFY counterpart
> which is calling fprintf to send to stderr.
>
> So ok let's limit this mode to glibcxx_assert.
>
>> The idea was to just add really cheap checks and abort  :-(
>>
>> Have you compared codegen with and without assertion mode? How much
>> more code is added to member functions like operator[] that must be
>> inlined for good performance?  Is it likely to affect inlining
>> decisions?
>>
>> I suspect it will have a much bigger impact than if we just use
>> __builtin_abort() as I made it do originally.
> I think that impact on compiled code depends more on the assert
> condition than on the code executed when this assertion happens to be
> false. But I haven't check it and will try.
>
> In the attached patch I eventually:
> - Move assertion macros in debug/assertions.h, it sounds like the right
> place for those.
> - Complete implementation of assertion checks by using __valid_range
> function. All checks I can think of are now in place. I still need to
> compare with google branch.
>
> Note that for the latter, condition is still evaluated in O(1).
> __valid_range detects iterator issues without looping through them.
> __valid_range, by considering iterator category, also make those macros
> usable in any container.
>
> François
>
Here is a last version I think.

    I completed the debug light mode by adding some check on iterator
ranges.

    Even if check are light I made some changes to make sure that
internally vector is not using methods instrumented with those checks.
This is to make sure checks are not done several times. Doing so also
simplify normal mode especially when using insert range, there is no
need to check if parameters are integers or not.

    I also introduce some __builtin_expect to make sure compiler will
prefer the best path.

    I didn't manage to check result on generated code. I am pretty sure
there will be an impact, you can't run more code without impact. But
that is a known drawback of debug mode, light or not, we just need to
minimize it. Mostly by making sure that checks are done only once.

    It would be great to have it for gcc 6.0. I am working on the same
for other containers.

François

-------------- next part --------------
A non-text attachment was scrubbed...
Name: vector_debug.patch
Type: text/x-patch
Size: 22001 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/libstdc++/attachments/20151115/eaf53176/attachment.bin>