Re: [PATCH] libstdc++: Check [ptr,end) and [ptr,ptr+n) ranges with _GLIBCXX_ASSERTIONS

["François Dumont" <frs.dumont@gmail.com>]

Hi

     On a related subject I am waiting for some feedback on:

https://gcc.gnu.org/pipermail/libstdc++/2021-August/053005.html


On 11/10/21 6:49 pm, Jonathan Wakely wrote:
> This enables lightweight checks for the __glibcxx_requires_valid_range
> and __glibcxx_requires_string_len macros  when _GLIBCXX_ASSERTIONS is
> defined.  By using __builtin_object_size we can check whether the end of
> the range is part of the same object as the start of the range, and
> detect problems like in PR 89927.
>
> libstdc++-v3/ChangeLog:
>
> 	* include/debug/debug.h (__valid_range_p, __valid_range_n): New
> 	inline functions using __builtin_object_size to check ranges
> 	delimited by pointers.
> 	[_GLIBCXX_ASSERTIONS] (__glibcxx_requires_valid_range): Use
> 	__valid_range_p.
> 	[_GLIBCXX_ASSERTIONS] (__glibcxx_requires_string_len): Use
> 	__valid_range_n.
>
>
> The first patch allows us to detect bugs like string("foo", "bar"),
> like in PR 89927. Debug mode cannot currently detect this. The new
> check uses the compiler built-in to detect when the two arguments are
> not part of the same object. This assumes we're optimizing and the
> compiler knows the values of the pointers. If it doesn't, then the
> function just returns true and should inline to nothing.

I see, it does not detect that input pointers are unrelated but as they 
are the computed size is >= __sz.

Isn't it UB to compare unrelated pointers ?

> I would like to also enable that for Debug Mode, otherwise we have
> checks that work for _GLIBCXX_ASSERTIONS but not for _GLIBCXX_DEBUG. I
> tried to make that work with the second patch attached to this mail,
> but it doesn't abort for the example in PR 89927. I think puttingthe
> checks inside the "real" debug checking functions is too many levels
> of inlining and the compiler "forgets" the pointer values.
>
> I think the first patch is worth committing. It should add no overhead
> for optimized builds, and diagnoses some bugs that we do not diagnose
> today. I'm less sure about the second, since it doesn't actually help.
> Maybe the second one should wait for Siddhesh's
> __builtin_dynamic_object_size to land on trunk.
>
> Taking this idea further, we could do something similar for
> __glibcxx_requires_string, which is currently almost useless (it only
> checks if the pointer is null) but could be changed to use
> __valid_range_n(_String, char_traits<...>::length(_String))
> so that we can diagnose non-null terminated strings (because the
> length that char-traits would give us would be larger than the size
> that __builtin_object_size would give us).
>
> Thoughts?
>
>