On 14/10/21 7:43 pm, Jonathan Wakely wrote:
On Thu, 14 Oct 2021 at 18:11, François Dumont <frs.dum...@gmail.com> wrote:
Hi
On a related subject I am waiting for some feedback on:
https://gcc.gnu.org/pipermail/libstdc++/2021-August/053005.html
I'm concerned that this adds too much overhead for the
_GLIBCXX_ASSERTIONS case. It adds function calls which are not
necessarily inlined, and which perform arithmetic and comparisons on
the arguments. That has a runtime cost which is non-zero.
I thought that limiting the checks to __valid_range would be fine for
_GLIBCXX_ASSERTIONS. If you do not want any overhead you just don't
define it.
The patches I sent in this thread have zero runtime cost, because they
use the compiler built-in which compiles away to nothing if the sizes
aren't known.
I'll try to find out if it can help for the test case on std::copy which
I was adding with my proposal.
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 ?
Yes, and my patch doesn't compare any pointers, does it?
+ __UINTPTR_TYPE__ __f = (__UINTPTR_TYPE__)__first;
+ __UINTPTR_TYPE__ __l = (__UINTPTR_TYPE__)__last;
+ if (const std::size_t __sz = __builtin_object_size(__first, 3))
+ return __f <= __l && (__l - __f) <= __sz;
Isn't it a comparison ?
But maybe this is what the previous cast is for, I never understood it.
Note that those cast could be moved within the if branch, even if I
guess that the compiler does it.
