linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: "Maciej W. Rozycki" <macro@linux-mips.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	the arch/x86 maintainers <x86@kernel.org>,
	linux-mips@vger.kernel.org, Randy Dunlap <rdunlap@infradead.org>,
	Denys Vlasenko <dvlasenk@redhat.com>
Subject: Re: [PATCHSET] saner elf compat
Date: Wed, 23 Dec 2020 07:03:20 +0000	[thread overview]
Message-ID: <20201223070320.GW3579531@ZenIV.linux.org.uk> (raw)
In-Reply-To: <alpine.LFD.2.21.2012160924010.2104409@eddie.linux-mips.org>

[Denys Vlasenko cc'd]

On Wed, Dec 16, 2020 at 09:44:53AM +0000, Maciej W. Rozycki wrote:
> On Wed, 16 Dec 2020, Al Viro wrote:
> 
> > >  It may be worth pushing through GDB's gdb.threads/tls-core.exp test case, 
> > > making sure no UNSUPPORTED results have been produced due to resource 
> > > limits preventing a core from being dumped (and no FAILs, of course), with 
> > > o32/n32 native GDB.  This should guarantee our output is still as expected 
> > > by an interpreter.  Sadly I'm currently not set up for such testing though 
> > > eventually I mean to.
> > 
> > Umm...  What triple does one use for n32 gdb?
> 
>  I don't think there's a standardised one, just configure with CC/CXX set 
> for n32 compilation, e.g.:
> 
> $ /path/to/configure CC="gcc -mabi=n32" CXX="g++ -mabi=n32"
> 
> (and any other options set as usually).  This has to be with CC/CXX rather 
> than CFLAGS/CXXFLAGS so that it is guaranteed to be never overridden with 
> any logic that might do any fiddling with compilation options.  This will 
> set up the test suite accordingly.
> 
>  NB this may already be the compiler's default, depending on how it was 
> configured, i.e. if `--with-abi=n32' was used, in which case no extra 
> options will be required.  I don't know if any standard MIPS distribution 
> does it though; 64-bit MIPS/Debian might.  This will be reported with `gcc 
> --help -v', somewhere along the way.
> 
>  Let me know if there are issues with this approach.

One issue is that testsuite doesn't care about $CC, $CFLAGS or anything
of that sort.  What I'd done was
cat >~/bin/cc-n32 <<'EOF'
#!/bin/sh
exec /usr/bin/gcc -mabi=n32 "$@"
EOF
chmod +x ~/bin/cc-n32
and add CC_FOR_TARGET="/home/al/bin/cc-n32" in RUNTESTFLAGS.

With that it works.  Moreover, it fixes a test failure on mainline.
Mainline kernel (5.10, same behaviour as debian/buster mips64el one):
Test run by al on Tue Dec 22 21:23:09 2020
Native configuration is mips64el-unknown-linux-gnuabin32

                === gdb tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/al/binutils-gdb/gdb/testsuite/config/unix.exp as tool-and-target-specific interface file.
Running /home/al/binutils-gdb/gdb/testsuite/gdb.threads/tls-core.exp ...
FAIL: gdb.threads/tls-core.exp: native: print thread-local storage variable
                === gdb Summary ===

# of expected passes            5
# of unexpected failures        1

vfs.git #work.elf-compat:
Test run by al on Tue Dec 22 21:31:14 2020
Native configuration is mips64el-unknown-linux-gnuabin32

                === gdb tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/al/binutils-gdb/gdb/testsuite/config/unix.exp as tool-and-target-specific interface file.
Running /home/al/binutils-gdb/gdb/testsuite/gdb.threads/tls-core.exp ...

                === gdb Summary ===

# of expected passes            6


Which is bloody embarrassing, since I'd completely missed the
behaviour change - this series was supposed to be an equivalent
transformation.

Anyway, the minimal patch fixing that failure is this one-liner and
unlike the elf-compat series it's trivial to backport:

[mips] fix n32 coredump breakage

Back in 2012, 49ae4d4b113b ("coredump: add a new elf note with siginfo
of the signal") has introduced a new ELF coredump note - NT_FILE.  It contains
a mix of strings and addresses, and addresses are 32bit for 32bit targets
and 64bit for 64bit ones.  Eventually gdb has come to use it.

Biarch targets had been taken care of from the very beginning - the
same commit has added a macro (user_long_t) with default being long
and fs/compat_binfmt_elf.c overriding it to compat_long_t.

Unfortunately, Denis had missed the mips weirdness.  As the result,
on mips64 both o32 and n32 ended up using 64-bit layout.  readelf(1)
is not happy.  More importantly, neither is gdb(1); as the matter
of fact, gdb.thread/tls-core.exp kept complaining.  Note that gcore(1)
is using 32bit layout for n32 case - it's only the kernel n32 coredumps
that get broken NT_FILE note.

NOTE: similar patch is almost certainly needed for o32; I have only
tested it with n32 gdb, though.

Fixes: 49ae4d4b113b ("coredump: add a new elf note with siginfo of the signal")
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/arch/mips/kernel/binfmt_elfn32.c b/arch/mips/kernel/binfmt_elfn32.c
index 6ee3f7218c67..c073136968e8 100644
--- a/arch/mips/kernel/binfmt_elfn32.c
+++ b/arch/mips/kernel/binfmt_elfn32.c
@@ -103,4 +103,6 @@ jiffies_to_old_timeval32(unsigned long jiffies, struct old_timeval32 *value)
 #undef ns_to_kernel_old_timeval
 #define ns_to_kernel_old_timeval ns_to_old_timeval32
 
+#define user_long_t compat_long_t
+
 #include "../../../fs/binfmt_elf.c"

  parent reply	other threads:[~2020-12-23  7:04 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-03 21:45 [PATCHSET] saner elf compat Al Viro
2020-12-03 21:46 ` [PATCH 01/10] binfmt_elf: partially sanitize PRSTATUS_SIZE and SET_PR_FPVALID Al Viro
2020-12-03 21:46   ` [PATCH 02/10] elf_prstatus: collect the common part (everything before pr_reg) into a struct Al Viro
2020-12-03 21:46   ` [PATCH 03/10] [elfcore-compat][amd64] clean PRSTATUS_SIZE/SET_PR_FPVALID up properly Al Viro
2020-12-03 21:46   ` [PATCH 04/10] mips binfmt_elf*32.c: use elfcore-compat.h Al Viro
2020-12-03 21:46   ` [PATCH 05/10] mips: kill unused definitions in binfmt_elf[on]32.c Al Viro
2020-12-03 21:46   ` [PATCH 06/10] mips: KVM_GUEST makes no sense for 64bit builds Al Viro
2020-12-03 21:46   ` [PATCH 07/10] mips compat: don't bother with ELF_ET_DYN_BASE Al Viro
2020-12-03 21:46   ` [PATCH 08/10] mips: don't bother with ELF_CORE_EFLAGS Al Viro
2020-12-03 21:46   ` [PATCH 09/10] mips compat: switch to compat_binfmt_elf.c Al Viro
2020-12-03 21:46   ` [PATCH 10/10] Kconfig: regularize selection of CONFIG_BINFMT_ELF Al Viro
2020-12-03 22:09 ` [PATCHSET] saner elf compat Linus Torvalds
2020-12-03 23:03   ` Al Viro
2020-12-06  3:23     ` Al Viro
2020-12-07  3:36       ` hpa
2020-12-07 18:01     ` Maciej W. Rozycki
2020-12-16  3:01       ` Al Viro
2020-12-16  9:44         ` Maciej W. Rozycki
2020-12-22 20:04           ` Al Viro
2020-12-22 21:38             ` Al Viro
2020-12-22 22:57               ` Al Viro
2020-12-23  7:03           ` Al Viro [this message]
2020-12-23  7:12             ` Al Viro
2020-12-24 19:44               ` [RFC][PATCH] NT_FILE/NT_SIGINFO breakage on mips compat coredumps Al Viro
2020-12-29 15:09                 ` Thomas Bogendoerfer
2020-12-15 19:54     ` [PATCHSET] saner elf compat Thomas Bogendoerfer

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=20201223070320.GW3579531@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=dvlasenk@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=macro@linux-mips.org \
    --cc=rdunlap@infradead.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tsbogend@alpha.franken.de \
    --cc=x86@kernel.org \
    /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 NNTP newsgroup(s).