From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3083 invoked by alias); 19 Apr 2017 09:15:38 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 2931 invoked by uid 89); 19 Apr 2017 09:15:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.6 required=5.0 tests=BAYES_00,FREEMAIL_FROM,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=operate, capping X-HELO: mail-wm0-f66.google.com Received: from mail-wm0-f66.google.com (HELO mail-wm0-f66.google.com) (74.125.82.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 19 Apr 2017 09:15:29 +0000 Received: by mail-wm0-f66.google.com with SMTP id o81so3797808wmb.0 for ; Wed, 19 Apr 2017 02:15:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=MOVvmSaJAC6tEPGfJRz0SudKzqJq1PWLsNxSIwn5MZU=; b=B4nVh84A81FDazxAGSfDfvZB+52SKJ/UUne7lf4QQzVdefr5SLoa1h7AxzkBq4XcmE 2mE+gZo3558xnVF4o5NbQkVt2G1METmGA5JOcbTlakVi5P0z2kF69dymqE6SyAjz/rCL HSKfz0J3xYmzUs3p1qR08ns/hJN5Ssrk2eAY47Gep3Ir2d7vGy+BfBxzuAuhoYrt4DEF YIuVIz1+/b0tUczDD2WXisdzk9PgX6Nxa+8eillj236poae0kJ0nVf+f5YALmbllgkRT mZ+178ggAwGsLAgx0guhJ8V5LiPlt7IZxXf/g5blS0lSISaychPUMoC2sBV3JPKobCaM HgEg== X-Gm-Message-State: AN3rC/4xkwcTxc46ywDGSBN3KqwWDZDYOBkCBG3pAzQ55XoF21K1R0uw VddfTRhzks2CLw== X-Received: by 10.28.221.212 with SMTP id u203mr1933278wmg.101.1492593329397; Wed, 19 Apr 2017 02:15:29 -0700 (PDT) Received: from E107787-LIN ([194.214.185.158]) by smtp.gmail.com with ESMTPSA id l141sm2917794wma.32.2017.04.19.02.15.28 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Wed, 19 Apr 2017 02:15:28 -0700 (PDT) From: Yao Qi To: Andreas Arnez Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 2/9] Fix size capping in write_pieced_value References: <1491586736-21296-1-git-send-email-arnez@linux.vnet.ibm.com> <1491586736-21296-3-git-send-email-arnez@linux.vnet.ibm.com> <86r30wafft.fsf@gmail.com> Date: Wed, 19 Apr 2017 09:15:00 -0000 In-Reply-To: (Andreas Arnez's message of "Thu, 13 Apr 2017 18:35:04 +0200") Message-ID: <86k26gahcg.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2017-04/txt/msg00552.txt.bz2 Andreas Arnez writes: > OK, I guess the commit message should be improved a bit. How about > this? > Hi Andreas, The description to the logic can go to comments, so that we don't need to do "git blame/log" to understand the code. > A field f in a structure composed of DWARF pieces may be located in > multiple pieces, where the first and last of those may contain bits > from other fields as well. So when writing to f, the beginning of the > first and the end of the last of those pieces may have to be skipped. > But the logic in write_pieced_value for handling one of those pieces > is flawed when the first and last piece are the same, i.e., f is > contained in a single piece: > > < - - - - - - - - - piece_size - - - - - - - - - -> > +-------------------------------------------------+ > | skipped_bits | f_bits | / / / / / / / / / / | > +-------------------------------------------------+ > > The current logic determines the size of the sub-piece to operate on > by limiting the piece size to the bit size of f and then subtracting > the skipped bits: > > max (piece_size, f_bits) - skipped_bits > > Instead of: > > max (piece_size - skipped_bits, f_bits) > Given this example, the result is the same, which is "piece_size - skipped_bits", am I missing something? > So the resulting sub-piece size is corrupted, leading to wrong > handling of this piece in write_pieced_value. > >> >>> logic in write_pieced_value for handling this is flawed when there are >>> actually bits to skip at the beginning of the first piece: it truncates >>> the piece size towards the end *before* accounting for the skipped bits >>> at the beginning instead of the other way around. >>> >>> Note that the same bug was already found in read_pieced_value and fixed >>> there (but not in write_pieced_value), see PR 15391. >> >> Can we share the code in write_pieced_value and read_pieced_value? The >> code computing offsets and bits should be shared. > > Yes. I have another patch (not posted yet) that merges these two > functions. I moved that towards the end of the patch series, so the > individual fixes can be incremental. > I'd like to merge the code first, then don't need to fix the same problem in two functions read_pieced_value and write_pieced_value (your patch 4/9 ~ 9/9 touches both two functions). >> Also, we need more comments in the code to explain these offsets and >> bits, a diagram about the relationships of these bits and offsets is >> quite helpful. > > OK. Some of the offset variables are removed by my patches, so I guess > I'll postpone that to the merged version. I'll see what I can come up > with and include it in v2. Please include it in V2. --=20 Yao (=E9=BD=90=E5=B0=A7)