review request [FT32] support for FT32B processor - part 1

review request [FT32] support for FT32B processor - part 1

[James Bowman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="us-ascii", Size: 6766 bytes --]

Please can somebody review this short patch - the binutils part of it
is approved, so I am now waiting on gdb approval.

https://sourceware.org/ml/gdb-patches/2017-10/msg00091.html

Thanks.
From gdb-patches-return-142346-listarch-gdb-patches=sources.redhat.com@sourceware.org Sat Oct 07 16:45:51 2017
Return-Path: <gdb-patches-return-142346-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Delivered-To: listarch-gdb-patches@sources.redhat.com
Received: (qmail 105653 invoked by alias); 7 Oct 2017 16:45:51 -0000
Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <gdb-patches.sourceware.org>
List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: gdb-patches-owner@sourceware.org
Delivered-To: mailing list gdb-patches@sourceware.org
Received: (qmail 105627 invoked by uid 89); 7 Oct 2017 16:45:49 -0000
Authentication-Results: sourceware.org; auth=none
X-Virus-Found: No
X-Spam-SWARE-Status: No, score=-1.9 required=5.0 testsºYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Hence, locating, serious
X-HELO: mx1.redhat.com
Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 07 Oct 2017 16:45:48 +0000
Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])	(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))	(No client certificate requested)	by mx1.redhat.com (Postfix) with ESMTPS id C5BE720271;	Sat,  7 Oct 2017 16:45:46 +0000 (UTC)
DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com C5BE720271
Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spfúil smtp.mailfrom=kevinb@redhat.com
Received: from pinnacle.lan (ovpn-116-131.phx2.redhat.com [10.3.116.131])	by smtp.corp.redhat.com (Postfix) with ESMTPS id 9D0335D75C;	Sat,  7 Oct 2017 16:45:46 +0000 (UTC)
Date: Sat, 07 Oct 2017 16:45:00 -0000
From: Kevin Buettner <kevinb@redhat.com>
To: gdb-patches@sourceware.org
Subject: Re: [PATCH 1/2] GDB test suite: Add helper for locating core files
Message-ID: <20171007094545.1bba5c51@pinnacle.lan>
In-Reply-To: <1505760152-28775-2-git-send-email-arnez@linux.vnet.ibm.com>
References: <1505760152-28775-1-git-send-email-arnez@linux.vnet.ibm.com>	<1505760152-28775-2-git-send-email-arnez@linux.vnet.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
X-IsSubscribed: yes
X-SW-Source: 2017-10/txt/msg00153.txt.bz2
Content-length: 3806

On Mon, 18 Sep 2017 20:41:51 +0200
Andreas Arnez <arnez@linux.vnet.ibm.com> wrote:

> The test suite contains a convenience proc `core_find' that basically
> performs two tasks:
>
> 1. Run a given command (expecting it to dump core).
> 2. Locate the core dump generated by the command, and return it.
>
> However, some test cases just need the second part, because they run the
> command in a different way.  Hence they implement their own logic for
> locating the core dump.
>
> This change replaces the instances of core dump retrieval logic by
> invocations of a new separate proc.  And it renames `core_find' to
> `run_and_get_core', to reduce confusion.
>
> gdb/testsuite/ChangeLog:
>
> 	* lib/gdb.exp (exec_in_dir): New helper proc.
> 	(core_find): Rename to...
> 	(run_and_get_core): ...this.  Remove the deletefiles argument.
> 	Split out the logic for locating the dump into...
> 	(find_core): ...this new proc.
> 	* gdb.arch/s390-multiarch.exp: Adjust invocations of core_find.
> 	* gdb.base/break-interp.exp: Likewise.
> 	* gdb.base/corefile.exp: Likewise.
> 	* gdb.threads/corethreads.exp: Likewise.
> 	* gdb.arch/s390-vregs.exp: Exploit find_core.
> 	* gdb.base/bigcore.exp: Likewise.

Hi Andreas,

I've looked your patches over and for the most part, I like them.
They better consolidate the logic for finding core files to
lib/gdb.exp.

However, one serious problem is that your rewrite of code in
lib/gdb.exp removes support for the handling of remote core files.

E.g. when I test your patch on my x86-64 linux box using the
following command...

    make check RUNTESTFLAGS="--target_board=native-gdbserver"

...I see 32 fewer passes than before and also one more known failure.

Here are the passes that no longer occur when using your patch:

    PASS: gdb.base/corefile.exp: args: -core=corefile.core
    PASS: gdb.base/corefile.exp: args: execfile -core=corefile.core
    PASS: gdb.base/corefile.exp: core-file command
    PASS: gdb.base/corefile.exp: print coremaker_data
    PASS: gdb.base/corefile.exp: print coremaker_bss
    PASS: gdb.base/corefile.exp: print coremaker_ro
    PASS: gdb.base/corefile.exp: print func2::coremaker_local
    PASS: gdb.base/corefile.exp: $_exitsignal prints SIGABRT (6)
    PASS: gdb.base/corefile.exp: $_exitcode is void
    PASS: gdb.base/corefile.exp: backtrace in corefile.exp
    PASS: gdb.base/corefile.exp: up in corefile.exp
    PASS: gdb.base/corefile.exp: accessing original mmap data in core file
    PASS: gdb.base/corefile.exp: accessing mmapped data in core file
    PASS: gdb.base/corefile.exp: up in corefile.exp (reinit)
    PASS: gdb.base/corefile.exp: core
    PASS: gdb.base/corefile.exp: run: load core again
    PASS: gdb.base/corefile.exp: run: sanity check we see the core file
    PASS: gdb.base/corefile.exp: run: with core
    PASS: gdb.base/corefile.exp: run: core file is cleared
    PASS: gdb.base/corefile.exp: quit with a process
    PASS: gdb.base/corefile.exp: quit with processes: n
    PASS: gdb.base/corefile.exp: no question: load core
    PASS: gdb.base/corefile.exp: quit with a core file
    PASS: gdb.base/corefile.exp: core-file warning-free
    PASS: gdb.threads/corethreads.exp: load core
    PASS: gdb.threads/corethreads.exp: sanity check we see the core file
    PASS: gdb.threads/corethreads.exp: print pthread_t of thread0
    PASS: gdb.threads/corethreads.exp: print pthread_t of thread1
    PASS: gdb.threads/corethreads.exp: thread0 found
    PASS: gdb.threads/corethreads.exp: thread1 found
    PASS: gdb.threads/corethreads.exp: no other thread found

Instead, several warnings are now printed instead:

    WARNING: Can not generate core dump on remote target.

If you can restore support for handling of remote core files, I'd very
much like to review this patch again.

Thanks,

Kevin