From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 37017 invoked by alias); 16 Oct 2017 12:03:20 -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 36642 invoked by uid 89); 16 Oct 2017 12:03:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-6.9 required=5.0 tests=BAYES_00,GIT_PATCH_1,SPF_PASS autolearn=ham version=3.3.2 spammy=HContent-Transfer-Encoding:8bit 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; Mon, 16 Oct 2017 12:03:13 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 869A8ABE8; Mon, 16 Oct 2017 12:03:11 +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> <2903a0d3-a2f1-791e-bff9-0d5939832dfa@suse.cz> <190c5684-631c-a7a0-631e-c1296081e3f0@redhat.com> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <4f7dc604-8b86-3f81-6d3c-5aaa3d46ac7b@suse.cz> Date: Mon, 16 Oct 2017 12:15:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <190c5684-631c-a7a0-631e-c1296081e3f0@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2017-10/txt/msg00987.txt.bz2 On 10/13/2017 04:59 PM, Jeff Law wrote: > On 10/13/2017 07:02 AM, Martin Liška wrote: >> 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? > It doesn't look valid to me. I'll dig into it. > > In general the sbitmap interface requires callers to DTRT -- failure can > easily lead to an out of bounds read or write. It's one of the things I > really dislike about the sbitmap implementation. > > So it's safe to assume that I'm fully supportive of adding more testing > to catch this kind thing. > > Jeff > Good. Should I prepare fix for the ICE I mentioned or have you been working on that? Thanks, Martin