From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27371 invoked by alias); 26 May 2017 15:49:08 -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 27059 invoked by uid 89); 26 May 2017 15:49:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=assurance, resort X-HELO: mail-wm0-f47.google.com Received: from mail-wm0-f47.google.com (HELO mail-wm0-f47.google.com) (74.125.82.47) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 26 May 2017 15:49:06 +0000 Received: by mail-wm0-f47.google.com with SMTP id 7so130971914wmo.1 for ; Fri, 26 May 2017 08:49:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=d4VpEc8UZmkoA/J3xGA39cuDo0HHhA9nP3WPkez2TEM=; b=tP/0rkBwGTGJS14s8J91XE3GvwPmRn6vQf9pJ5jUVKSe0loaW0iMKerbdwc2vdVSND lLuk0NuP20uHVm3lSK9VwMIWnFHFtAfj0xZIzf1JmAG8ciNZUmdWJlbXNMallJYiabOu a857GftlSC9ThXNa4RnMhPC61OXTD71zd4eoxOJ8/3ZIYybcLK0H/ZpJL5eIZ8Iwohvt X6IeLcz/oHGaUsZ1vkTEaQ/1vifoXg+hYdWQDj+YOkIewNuTxMIkV8JwUC1MkOE3Nblf RKdbc+ulnGQcubDRiSCvj1D9l3o6+vAhe8MTGsl00Hr5SFr6sUrwUosS407mvPdh+fFa 3j1g== X-Gm-Message-State: AODbwcAjLkwmg29gE5EF5zvBtanB4tISO+56wvQqCMGXjhmHuY0H+vmK IOlHuygsutYJOEih X-Received: by 10.28.211.5 with SMTP id k5mr5190562wmg.11.1495813747836; Fri, 26 May 2017 08:49:07 -0700 (PDT) Received: from [192.168.0.102] ([37.189.166.198]) by smtp.gmail.com with ESMTPSA id m11sm2937064wmg.34.2017.05.26.08.49.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 26 May 2017 08:49:07 -0700 (PDT) Subject: Re: [PATCH 3/11] Add MIPS_MAX_REGISTER_SIZE (2/4) To: Alan Hayward References: <3C00280E-37C9-4C0A-9DA6-F3B9DB1A6E8F@arm.com> <86y3v7uf9j.fsf@gmail.com> <806B436F-EFA1-4200-AC54-9036D166C9B9@arm.com> <867f1m8nhm.fsf@gmail.com> <8637bx9jsw.fsf@gmail.com> <78A7E8EA-7203-44DF-B7FD-63E75A5ECEF5@arm.com> <540372d8-efc3-f842-5cac-cd813bacc3f5@redhat.com> <4F90CD36-759D-4BDA-BFEC-8DD86F44A0B7@arm.com> <40597975-9458-e9af-8915-9d303bb1ed98@redhat.com> <5A105765-C70D-413C-BB35-50BAA5FD5865@arm.com> <73b5b4f8-065b-7102-a9d8-0b909b1eb124@redhat.com> <86wp95p3ev.fsf@gmail.com> <2b6c26fe-be7e-e219-7c6a-7fb7425f7dc1@redhat.com> <73867a49-889b-171d-db60-d545da68b8dd@redhat.com> Cc: Yao Qi , "gdb-patches@sourceware.org" , nd From: Pedro Alves Message-ID: <6e87b30d-b3fd-c25c-501e-290abb3bc60c@redhat.com> Date: Fri, 26 May 2017 15:49:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-05/txt/msg00566.txt.bz2 On 05/26/2017 04:30 PM, Alan Hayward wrote: > >> On 26 May 2017, at 11:26, Pedro Alves wrote: > > >> Another issue with the test is that by relying on >> extract_(un)signed_integer, the test can't notice if the >> zero / sign extension in copy_integer_bytes filled the right >> number of bytes: >> >>> + do_cint_test (0xffffffffffffbeef, 2, 0xbeef, 3); >> >> I.e., above, the last test _must_ not write more than >> two bytes to the destination, so those leading >> 0xFFs before "beef" are artificial. >> >> Passing ULONGEST as destination value avoids that problem, making >> you write that last test as: >> >> do_cint_test (0x000000000000beef, 2, 0xbeef, 3); >> > > To make that work, I also had to make extract use the unsigned > version. So now all the stores and extracts are working on unsigned data. Makes sense. > All the above fixed up. > After doing this, I noticed it was easier to combine the two test functions > into one. Agreed. > > > Tested on a --enable-targets=all build using make check with board files > unix and native-gdbserver. > I do not have a MIPS machine to test on. I think the unit tests now give us sufficient assurance. > Ok to commit? Yes, with nit below addressed. > +static void > +copy_integer_to_size_test () > +{ > + /* Destination is bigger than the source, which has the signed bit unset. */ > + do_cint_test (0x12345678, 0x12345678, 8, 0x12345678, 4); > + do_cint_test (0x345678, 0x345678, 8, 0x12345678, 3); > + do_cint_test (0x5678, 0x5678, 2, 0x12345678, 3); The third test does not agree with the comment - destination is smaller than source. > + > + /* Destination is bigger than the source, which has the signed bit set. */ > + do_cint_test (0xdeadbeef, 0xffffffffdeadbeef, 8, 0xdeadbeef, 4); > + do_cint_test (0xadbeef, 0xffffffffffadbeef, 8, 0xdeadbeef, 3); > + do_cint_test (0xbeef, 0xbeef, 2, 0xdeadbeef, 3); Ditto. Re-sort to a separate block, and/or update comments, and this is good to go. Thanks, Pedro Alves