From patchwork Mon Oct 11 16:49:45 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 46107 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7DAC23857C4A for ; Mon, 11 Oct 2021 16:50:22 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7DAC23857C4A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1633971022; bh=sHkEKjgd5B0XJLfRnbtHaIs66J38dNLl+MWLrOJLHLg=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=X1rQiYuSbEw4m0UaO7ISoR6RCHKB2rYbMgHRnTszAu1jHso2mfMdZ01uTktyMKPFv J19RkzIDV6P4RSZPToUzWg94c1/W0FhJ3epguAf2PjkKtGPfQYdRh9dq21OuQL3swI d4XPb6JKbJHh4hAmFZr9MKrJvmzQTIxJil0rN+eQ= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 7F01C3858D28 for ; Mon, 11 Oct 2021 16:49:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7F01C3858D28 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-431-JCnca00SOBalPujWZRYPnQ-1; Mon, 11 Oct 2021 12:49:48 -0400 X-MC-Unique: JCnca00SOBalPujWZRYPnQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2F6AF835DE1; Mon, 11 Oct 2021 16:49:47 +0000 (UTC) Received: from localhost (unknown [10.33.37.44]) by smtp.corp.redhat.com (Postfix) with ESMTP id BBB0119C59; Mon, 11 Oct 2021 16:49:46 +0000 (UTC) Date: Mon, 11 Oct 2021 17:49:45 +0100 To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH] libstdc++: Check [ptr,end) and [ptr,ptr+n) ranges with _GLIBCXX_ASSERTIONS Message-ID: MIME-Version: 1.0 X-Clacks-Overhead: GNU Terry Pratchett X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-13.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jonathan Wakely via Gcc-patches From: Jonathan Wakely Reply-To: Jonathan Wakely Cc: =?iso-8859-1?q?Fran=E7ois?= Dumont Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" 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 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? commit b008cc08c6b05e32c896ed6e5a3e289ccf8f3c91 Author: Jonathan Wakely Date: Mon Oct 11 15:58:43 2021 libstdc++: Check [ptr,end) and [ptr,ptr+n) ranges with _GLIBCXX_ASSERTIONS 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. diff --git a/libstdc++-v3/include/debug/debug.h b/libstdc++-v3/include/debug/debug.h index 1db5aa34c55..15df64f54d9 100644 --- a/libstdc++-v3/include/debug/debug.h +++ b/libstdc++-v3/include/debug/debug.h @@ -70,7 +70,11 @@ namespace __gnu_debug __UINTPTR_TYPE__ __l = (__UINTPTR_TYPE__)__last; if (const std::size_t __sz = __builtin_object_size(__first, 3)) return __f <= __l && (__l - __f) <= __sz; +#ifdef _GLIBCXX_DEBUG + return __f <= __l && static_cast(__f) == static_cast(__l); +#else return true; +#endif } #ifndef _GLIBCXX_DEBUG @@ -89,7 +93,11 @@ namespace __gnu_debug { if (const std::size_t __sz = __builtin_object_size(__first, 3)) return __n <= __sz; +#ifdef _GLIBCXX_DEBUG + return static_cast(__first) == static_cast(__n); +#else return true; +#endif } #endif } diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h index c0144ced979..ed11147b451 100644 --- a/libstdc++-v3/include/debug/helper_functions.h +++ b/libstdc++-v3/include/debug/helper_functions.h @@ -266,6 +266,12 @@ namespace __gnu_debug return __valid_range_aux(__first, __last, _Integral()); } + template + __attribute__((__always_inline__)) _GLIBCXX14_CONSTEXPR + inline bool + __valid_range(_Tp* __first, _Tp* __last) _GLIBCXX_NOEXCEPT + { return __gnu_debug::__valid_range_p(__first, __last); } + template bool __valid_range(const _Safe_iterator<_Iterator, _Sequence, _Category>&, @@ -285,6 +291,12 @@ namespace __gnu_debug __can_advance(_InputIterator, _Size) { return true; } + template + __attribute__((__always_inline__)) _GLIBCXX_CONSTEXPR + inline bool + __can_advance(_Tp* __p, _Size __n) _GLIBCXX_NOEXCEPT + { return __gnu_debug::__valid_range_n(__p, (std::size_t)__n); } + template bool diff --git a/libstdc++-v3/include/debug/macros.h b/libstdc++-v3/include/debug/macros.h index 9e1288cf4d9..5c655083cb3 100644 --- a/libstdc++-v3/include/debug/macros.h +++ b/libstdc++-v3/include/debug/macros.h @@ -461,6 +461,6 @@ _GLIBCXX_DEBUG_VERIFY(_This.get_allocator() == _Other.get_allocator(), \ #define __glibcxx_check_string(_String) _GLIBCXX_DEBUG_PEDASSERT(_String != 0) #define __glibcxx_check_string_len(_String,_Len) \ - _GLIBCXX_DEBUG_PEDASSERT(_String != 0 || _Len == 0) + _GLIBCXX_DEBUG_ASSERT(__gnu_debug::__valid_range_n(_String, _Len)) #endif diff --git a/libstdc++-v3/include/debug/stl_iterator.h b/libstdc++-v3/include/debug/stl_iterator.h index edeb42ebe98..65c1828d6e4 100644 --- a/libstdc++-v3/include/debug/stl_iterator.h +++ b/libstdc++-v3/include/debug/stl_iterator.h @@ -29,6 +29,7 @@ #ifndef _GLIBCXX_DEBUG_STL_ITERATOR_H #define _GLIBCXX_DEBUG_STL_ITERATOR_H 1 +#include #include namespace __gnu_debug diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc index 0128535135e..59382defe27 100644 --- a/libstdc++-v3/src/c++11/debug.cc +++ b/libstdc++-v3/src/c++11/debug.cc @@ -25,6 +25,7 @@ #include #include +#include #include #include #include diff --git a/libstdc++-v3/include/debug/debug.h b/libstdc++-v3/include/debug/debug.h index 116f2f023e2..1db5aa34c55 100644 --- a/libstdc++-v3/include/debug/debug.h +++ b/libstdc++-v3/include/debug/debug.h @@ -59,12 +59,46 @@ namespace __gnu_debug template struct _Safe_iterator; + +#ifdef _GLIBCXX_ASSERTIONS + template + __attribute__((__always_inline__)) + _GLIBCXX14_CONSTEXPR inline bool + __valid_range_p(_Tp* __first, _Tp* __last) _GLIBCXX_NOEXCEPT + { + __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; + return true; + } + +#ifndef _GLIBCXX_DEBUG + // __glibcxx_requires_valid_range uses this overload for non-pointers. + template + __attribute__((__always_inline__)) + _GLIBCXX14_CONSTEXPR inline bool + __valid_range_p(_Tp, _Tp) _GLIBCXX_NOEXCEPT + { return true; } +#endif + + template + _GLIBCXX14_CONSTEXPR __attribute__((__always_inline__)) + inline bool + __valid_range_n(_Tp* __first, std::size_t __n) _GLIBCXX_NOEXCEPT + { + if (const std::size_t __sz = __builtin_object_size(__first, 3)) + return __n <= __sz; + return true; + } +#endif } #ifndef _GLIBCXX_DEBUG # define __glibcxx_requires_cond(_Cond,_Msg) -# define __glibcxx_requires_valid_range(_First,_Last) +# define __glibcxx_requires_valid_range(_First,_Last) \ + __glibcxx_assert(__gnu_debug::__valid_range_p(_First, _Last)) # define __glibcxx_requires_can_increment(_First,_Size) # define __glibcxx_requires_can_increment_range(_First1,_Last1,_First2) # define __glibcxx_requires_can_decrement_range(_First1,_Last1,_First2) @@ -79,7 +113,8 @@ namespace __gnu_debug # define __glibcxx_requires_heap(_First,_Last) # define __glibcxx_requires_heap_pred(_First,_Last,_Pred) # define __glibcxx_requires_string(_String) -# define __glibcxx_requires_string_len(_String,_Len) +# define __glibcxx_requires_string_len(_String,_Len) \ + __glibcxx_assert(__gnu_debug::__valid_range_n(_String, _Len)) # define __glibcxx_requires_irreflexive(_First,_Last) # define __glibcxx_requires_irreflexive2(_First,_Last) # define __glibcxx_requires_irreflexive_pred(_First,_Last,_Pred)