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?
