From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7966 invoked by alias); 13 Oct 2017 13:02:36 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 7954 invoked by uid 89); 13 Oct 2017 13:02:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.2 spammy=3328, 2738 X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 13 Oct 2017 13:02:31 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 5BFC5AAC5; Fri, 13 Oct 2017 13:02:29 +0000 (UTC) Subject: Re: [PATCH] Fix bitmap_bit_in_range_p (PR tree-optimization/82493). To: Jeff Law , gcc-patches@gcc.gnu.org References: <9c9fd60f-cb7a-e702-aabb-9e31dca6a92a@suse.cz> <2215478f-6715-189a-e6a4-8d171901d31f@redhat.com> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <2903a0d3-a2f1-791e-bff9-0d5939832dfa@suse.cz> Date: Fri, 13 Oct 2017 13:03:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <2215478f-6715-189a-e6a4-8d171901d31f@redhat.com> Content-Type: multipart/mixed; boundary="------------CC4E405C1BEB1E6BFD3DBC2B" X-IsSubscribed: yes X-SW-Source: 2017-10/txt/msg00857.txt.bz2 This is a multi-part message in MIME format. --------------CC4E405C1BEB1E6BFD3DBC2B Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-length: 1857 On 10/12/2017 11:54 PM, Jeff Law wrote: > On 10/11/2017 12:13 AM, Martin Liška wrote: >> 2017-10-10 Martin Liska >> >> PR tree-optimization/82493 >> * sbitmap.c (bitmap_bit_in_range_p): Fix the implementation. >> (test_range_functions): New function. >> (sbitmap_c_tests): Likewise. >> * selftest-run-tests.c (selftest::run_tests): Run new tests. >> * selftest.h (sbitmap_c_tests): New function. > I went ahead and committed this along with a patch to fix the off-by-one > error in live_bytes_read. Bootstrapped and regression tested on x86. > > Actual patch attached for archival purposes. > > Jeff > Hello. I wrote a patch that adds various gcc_checking_asserts and I hit following: ./xgcc -B. /home/marxin/Programming/gcc/gcc/testsuite/gfortran.dg/char_result_12.f90 -c -O2 during GIMPLE pass: dse /home/marxin/Programming/gcc/gcc/testsuite/gfortran.dg/char_result_12.f90:7:0: program testat internal compiler error: in bitmap_check_index, at sbitmap.h:105 0x1c014c1 bitmap_check_index ../../gcc/sbitmap.h:105 0x1c01fa7 bitmap_bit_in_range_p(simple_bitmap_def const*, unsigned int, unsigned int) ../../gcc/sbitmap.c:335 0x1179002 live_bytes_read ../../gcc/tree-ssa-dse.c:497 0x117935a dse_classify_store ../../gcc/tree-ssa-dse.c:595 0x1179947 dse_dom_walker::dse_optimize_stmt(gimple_stmt_iterator*) ../../gcc/tree-ssa-dse.c:786 0x1179b6e dse_dom_walker::before_dom_children(basic_block_def*) ../../gcc/tree-ssa-dse.c:853 0x1a6f659 dom_walker::walk(basic_block_def*) ../../gcc/domwalk.c:308 0x1179cb9 execute ../../gcc/tree-ssa-dse.c:907 Where we call: Breakpoint 1, bitmap_bit_in_range_p (bmap=0x29d6cd0, start=0, end=515) at ../../gcc/sbitmap.c:335 335 bitmap_check_index (bmap, end); (gdb) p *bmap $1 = {n_bits = 256, size = 4, elms = {255}} Is it a valid call or should caller check indices? Martin --------------CC4E405C1BEB1E6BFD3DBC2B Content-Type: text/x-patch; name="0002-Add-gcc_checking_assert-for-sbitmap.c.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0002-Add-gcc_checking_assert-for-sbitmap.c.patch" Content-length: 6836 >From ba3d597be70b8329abafe92da868ab5250610840 Mon Sep 17 00:00:00 2001 From: marxin Date: Fri, 13 Oct 2017 13:39:08 +0200 Subject: [PATCH 2/2] Add gcc_checking_assert for sbitmap.c. --- gcc/sbitmap.c | 39 +++++++++++++++++++++++++++++++++++++++ gcc/sbitmap.h | 25 +++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/gcc/sbitmap.c b/gcc/sbitmap.c index fdcf5393e53..df933f6516c 100644 --- a/gcc/sbitmap.c +++ b/gcc/sbitmap.c @@ -180,6 +180,8 @@ sbitmap_vector_alloc (unsigned int n_vecs, unsigned int n_elms) void bitmap_copy (sbitmap dst, const_sbitmap src) { + gcc_checking_assert (src->size <= dst->size); + memcpy (dst->elms, src->elms, sizeof (SBITMAP_ELT_TYPE) * dst->size); } @@ -187,6 +189,8 @@ bitmap_copy (sbitmap dst, const_sbitmap src) int bitmap_equal_p (const_sbitmap a, const_sbitmap b) { + bitmap_check_sizes (a, b); + return !memcmp (a->elms, b->elms, sizeof (SBITMAP_ELT_TYPE) * a->size); } @@ -211,6 +215,8 @@ bitmap_clear_range (sbitmap bmap, unsigned int start, unsigned int count) if (count == 0) return; + bitmap_check_index (bmap, start + count - 1); + unsigned int start_word = start / SBITMAP_ELT_BITS; unsigned int start_bitno = start % SBITMAP_ELT_BITS; @@ -267,6 +273,8 @@ bitmap_set_range (sbitmap bmap, unsigned int start, unsigned int count) if (count == 0) return; + bitmap_check_index (bmap, start + count - 1); + unsigned int start_word = start / SBITMAP_ELT_BITS; unsigned int start_bitno = start % SBITMAP_ELT_BITS; @@ -324,6 +332,8 @@ bool bitmap_bit_in_range_p (const_sbitmap bmap, unsigned int start, unsigned int end) { gcc_checking_assert (start <= end); + bitmap_check_index (bmap, end); + unsigned int start_word = start / SBITMAP_ELT_BITS; unsigned int start_bitno = start % SBITMAP_ELT_BITS; @@ -467,6 +477,9 @@ bitmap_vector_ones (sbitmap *bmap, unsigned int n_vecs) bool bitmap_ior_and_compl (sbitmap dst, const_sbitmap a, const_sbitmap b, const_sbitmap c) { + bitmap_check_sizes (a, b); + bitmap_check_sizes (b, c); + unsigned int i, n = dst->size; sbitmap_ptr dstp = dst->elms; const_sbitmap_ptr ap = a->elms; @@ -489,6 +502,8 @@ bitmap_ior_and_compl (sbitmap dst, const_sbitmap a, const_sbitmap b, const_sbitm void bitmap_not (sbitmap dst, const_sbitmap src) { + bitmap_check_sizes (src, dst); + unsigned int i, n = dst->size; sbitmap_ptr dstp = dst->elms; const_sbitmap_ptr srcp = src->elms; @@ -510,6 +525,9 @@ bitmap_not (sbitmap dst, const_sbitmap src) void bitmap_and_compl (sbitmap dst, const_sbitmap a, const_sbitmap b) { + bitmap_check_sizes (a, b); + bitmap_check_sizes (b, dst); + unsigned int i, dst_size = dst->size; unsigned int min_size = dst->size; sbitmap_ptr dstp = dst->elms; @@ -537,6 +555,8 @@ bitmap_and_compl (sbitmap dst, const_sbitmap a, const_sbitmap b) bool bitmap_intersect_p (const_sbitmap a, const_sbitmap b) { + bitmap_check_sizes (a, b); + const_sbitmap_ptr ap = a->elms; const_sbitmap_ptr bp = b->elms; unsigned int i, n; @@ -555,6 +575,9 @@ bitmap_intersect_p (const_sbitmap a, const_sbitmap b) bool bitmap_and (sbitmap dst, const_sbitmap a, const_sbitmap b) { + bitmap_check_sizes (a, b); + bitmap_check_sizes (b, dst); + unsigned int i, n = dst->size; sbitmap_ptr dstp = dst->elms; const_sbitmap_ptr ap = a->elms; @@ -577,6 +600,9 @@ bitmap_and (sbitmap dst, const_sbitmap a, const_sbitmap b) bool bitmap_xor (sbitmap dst, const_sbitmap a, const_sbitmap b) { + bitmap_check_sizes (a, b); + bitmap_check_sizes (b, dst); + unsigned int i, n = dst->size; sbitmap_ptr dstp = dst->elms; const_sbitmap_ptr ap = a->elms; @@ -599,6 +625,9 @@ bitmap_xor (sbitmap dst, const_sbitmap a, const_sbitmap b) bool bitmap_ior (sbitmap dst, const_sbitmap a, const_sbitmap b) { + bitmap_check_sizes (a, b); + bitmap_check_sizes (b, dst); + unsigned int i, n = dst->size; sbitmap_ptr dstp = dst->elms; const_sbitmap_ptr ap = a->elms; @@ -620,6 +649,8 @@ bitmap_ior (sbitmap dst, const_sbitmap a, const_sbitmap b) bool bitmap_subset_p (const_sbitmap a, const_sbitmap b) { + bitmap_check_sizes (a, b); + unsigned int i, n = a->size; const_sbitmap_ptr ap, bp; @@ -636,6 +667,10 @@ bitmap_subset_p (const_sbitmap a, const_sbitmap b) bool bitmap_or_and (sbitmap dst, const_sbitmap a, const_sbitmap b, const_sbitmap c) { + bitmap_check_sizes (a, b); + bitmap_check_sizes (b, c); + bitmap_check_sizes (c, dst); + unsigned int i, n = dst->size; sbitmap_ptr dstp = dst->elms; const_sbitmap_ptr ap = a->elms; @@ -659,6 +694,10 @@ bitmap_or_and (sbitmap dst, const_sbitmap a, const_sbitmap b, const_sbitmap c) bool bitmap_and_or (sbitmap dst, const_sbitmap a, const_sbitmap b, const_sbitmap c) { + bitmap_check_sizes (a, b); + bitmap_check_sizes (b, c); + bitmap_check_sizes (c, dst); + unsigned int i, n = dst->size; sbitmap_ptr dstp = dst->elms; const_sbitmap_ptr ap = a->elms; diff --git a/gcc/sbitmap.h b/gcc/sbitmap.h index ff52e939bf3..a5ff0685e43 100644 --- a/gcc/sbitmap.h +++ b/gcc/sbitmap.h @@ -96,10 +96,29 @@ struct simple_bitmap_def /* Return the number of bits in BITMAP. */ #define SBITMAP_SIZE(BITMAP) ((BITMAP)->n_bits) +/* Verify that access at INDEX in bitmap MAP is valid. */ + +static inline void +bitmap_check_index (const_sbitmap map, int index) +{ + gcc_checking_assert (index >= 0); + gcc_checking_assert ((unsigned int)index < map->n_bits); +} + +/* Verify that bitmaps A and B have same size. */ + +static inline void +bitmap_check_sizes (const_sbitmap a, const_sbitmap b) +{ + gcc_checking_assert (a->n_bits == b->n_bits); +} + /* Test if bit number bitno in the bitmap is set. */ static inline SBITMAP_ELT_TYPE bitmap_bit_p (const_sbitmap map, int bitno) { + bitmap_check_index (map, bitno); + size_t i = bitno / SBITMAP_ELT_BITS; unsigned int s = bitno % SBITMAP_ELT_BITS; return (map->elms[i] >> s) & (SBITMAP_ELT_TYPE) 1; @@ -110,6 +129,8 @@ bitmap_bit_p (const_sbitmap map, int bitno) static inline void bitmap_set_bit (sbitmap map, int bitno) { + bitmap_check_index (map, bitno); + map->elms[bitno / SBITMAP_ELT_BITS] |= (SBITMAP_ELT_TYPE) 1 << (bitno) % SBITMAP_ELT_BITS; } @@ -119,6 +140,8 @@ bitmap_set_bit (sbitmap map, int bitno) static inline void bitmap_clear_bit (sbitmap map, int bitno) { + bitmap_check_index (map, bitno); + map->elms[bitno / SBITMAP_ELT_BITS] &= ~((SBITMAP_ELT_TYPE) 1 << (bitno) % SBITMAP_ELT_BITS); } @@ -148,6 +171,8 @@ static inline void bmp_iter_set_init (sbitmap_iterator *i, const_sbitmap bmp, unsigned int min, unsigned *bit_no ATTRIBUTE_UNUSED) { + bitmap_check_index (bmp, min); + i->word_num = min / (unsigned int) SBITMAP_ELT_BITS; i->bit_num = min; i->size = bmp->size; -- 2.14.2 --------------CC4E405C1BEB1E6BFD3DBC2B--