From: YunQiang Su <wzssyqa@gmail.com>
To: "Maciej W. Rozycki" <macro@orcam.me.uk>
Cc: Nick Clifton <nickc@redhat.com>, binutils@sourceware.org
Subject: Re: New failures for the mips64el-openbsd target
Date: Mon, 19 Jun 2023 15:12:15 +0800 [thread overview]
Message-ID: <CAKcpw6UUZMXXa1HrCJBZKeDDhk3x1U-=_67=tAsQ0XwZoUNkpw@mail.gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.21.2306161300440.64925@angie.orcam.me.uk>
Maciej W. Rozycki <macro@orcam.me.uk> 于2023年6月16日周五 21:33写道:
>
> On Fri, 16 Jun 2023, YunQiang Su wrote:
>
> > > It may well be that one of the YunQiang Su's patches I have reverted
> > > brough these back; a regression from his earlier commit(s) anyway.
> > >
> >
> > I run a test with the below commit, because the next commit of it is
> > my first commit to binutils.
> >
> > commit 2e6be59c8de57c32260771ac5307968d18793a0a (HEAD -> xxxxx)
> > Author: Nick Clifton <nickc@redhat.com>
> > Date: Thu Jul 25 13:05:27 2019 +0100
> >
> > Stop an illegal memory access by readelf when parsing a corrupt
> > MIPS binary file.
> >
> > PR 24837
> > * readelf.c (process_mips_specific): Check for buffer overflow
> > before reading reginfo information.
> >
> > The test fails do exist with this commit.
> >
> > So I think that it is a long time problem, and in fact my recent
> > commits resolve them.
>
> Good. Please post fixes then to regressions introduced with any changes
> of yours starting from commit 32f1c80375eb ("MIPS: support mips*64 as CPU
> and gnuabi64 as ABI") as a separate change (or multiple changes) from any
> fixes to pre-existing problems. The more self-contained a change is the
> more straightforward to review it is.
>
I don't think that 2 changes introduced any "regressions".
Yes, with these 2 changes, if binutils is configured with -
-target=mips64-linux-gnuabi64 or -target=mipsisa64r6-linux-gnu
will have some failures. But they are not "regressions", the real problem
is that the previous code is wrong at all.
Yes, I will fix these failures, that the patches do:
[PATCH v4 3/7] MIPS: Fix r6 testsuites
[PATCH v4 4/7] MIPS: Fix -gnuabi64 testsuite
> It's OK to fix regressions one by one with separate patches, or to only
> have an identical change bulk-applied across multiple files in a single
> patch.
>
> Overall changes that live in downstream repositories often have a long
> history behind them, which is not necessarily relevant at the upstream
> submission time. Consequently they may have to be merged, split, and/or
> have bits and pieces moved between changes to make them consistent from
> the upstream code's point of view. I've been through it myself across
> many projects and I am sure I'm no exception here.
>
> So please try and avoid submitting unrelated changes bundled together in
> a single patch, and of course regression-test each individual change in a
> patch set as they are incrementally applied.
>
> Also please try and separate changes to the MIPS subset of the testsuites
> from ones to their generic part, as those usually need to be reviewed by a
> general maintainer (unless say you're just about to remove a MIPS XFAIL),
> and also need to be verified across non-MIPS targets.
>
> > > I haven't run any verification on the reverts as I don't think there is
> > > any excuse for violating our policies and committing patches with no prior
> > > approval and/or with objections raised. I think let's wait and see how
> > > YunQiang handles it, and if it turns out to be a failure, then I'll handle
> > > it one way or another (maybe his earlier commits need to be reverted too).
> > >
> >
> > I will resend my patchset, of course with more code style checks.
>
> I look forward to that, thank you.
>
> Maciej
--
YunQiang Su
next prev parent reply other threads:[~2023-06-19 7:12 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-15 10:46 Nick Clifton
2023-06-15 14:43 ` Maciej W. Rozycki
2023-06-16 3:00 ` YunQiang Su
2023-06-16 13:33 ` Maciej W. Rozycki
2023-06-19 7:12 ` YunQiang Su [this message]
2023-06-19 7:23 ` YunQiang Su
2023-06-16 4:29 ` Alan Modra
2023-06-16 14:07 ` Maciej W. Rozycki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKcpw6UUZMXXa1HrCJBZKeDDhk3x1U-=_67=tAsQ0XwZoUNkpw@mail.gmail.com' \
--to=wzssyqa@gmail.com \
--cc=binutils@sourceware.org \
--cc=macro@orcam.me.uk \
--cc=nickc@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).