Bug 345763 - MIPS N32 ABI support
Summary: MIPS N32 ABI support
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (show other bugs)
Version: 3.10 SVN
Platform: Compiled Sources Linux
: NOR wishlist
Target Milestone: ---
Assignee: Petar Jovanovic
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-01 15:38 UTC by Crestez Dan Leonard
Modified: 2018-06-14 18:38 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
mips64n32-valgrind.diff (102.19 KB, patch)
2015-04-01 15:38 UTC, Crestez Dan Leonard
Details
mips64n32-valgrind-vex.diff (1.88 KB, patch)
2015-04-01 15:39 UTC, Crestez Dan Leonard
Details
mips64n32-V2-valgrind.diff (104.85 KB, patch)
2015-04-07 19:15 UTC, Crestez Dan Leonard
Details
mips64n32-V3-valgrind.diff (112.98 KB, patch)
2015-04-15 00:40 UTC, Crestez Dan Leonard
Details
mini-mips64n32-valgrind-v1.patch (54.29 KB, patch)
2015-04-23 10:20 UTC, Crestez Dan Leonard
Details
mini-mips64n32-valgrind-vex-v1.patch (496 bytes, patch)
2015-04-23 10:20 UTC, Crestez Dan Leonard
Details
test results (partial) (24.46 KB, text/x-log)
2015-04-23 10:22 UTC, Crestez Dan Leonard
Details
mini-mips64n32-valgrind-v2.patch (56.63 KB, patch)
2015-04-23 15:56 UTC, Crestez Dan Leonard
Details
test results v2 (36.23 KB, text/x-log)
2015-04-23 15:58 UTC, Crestez Dan Leonard
Details
valgrind-cavium-mini-mips64n32-v3.log (33.10 KB, text/x-log)
2015-04-23 19:48 UTC, Crestez Dan Leonard
Details
mini-mips64n32-valgrind-v3.patch (67.56 KB, patch)
2015-04-23 19:50 UTC, Crestez Dan Leonard
Details
mini-mips64n32-valgrind-v4.patch (69.01 KB, patch)
2015-07-20 14:25 UTC, Crestez Dan Leonard
Details
valgrind mips64n32 support on top of 3.11.0 (69.01 KB, patch)
2015-11-26 21:48 UTC, Crestez Dan Leonard
Details
valgrind vex mips64n32 support on top of 3.11.0 (496 bytes, patch)
2015-11-26 21:49 UTC, Crestez Dan Leonard
Details
n32 support valgrind patch rebased on 3.13 (68.99 KB, patch)
2017-07-27 08:41 UTC, Maran Pakkirisamy
Details
n32 support VEX patch rebased on 3.13 (499 bytes, patch)
2017-07-27 08:43 UTC, Maran Pakkirisamy
Details
fix name mismatch (673 bytes, patch)
2017-07-27 08:46 UTC, Maran Pakkirisamy
Details
fix building valgrind for N32 (1.44 KB, patch)
2017-07-27 08:49 UTC, Maran Pakkirisamy
Details
Introduce RegWord type (319.87 KB, patch)
2018-04-05 12:04 UTC, Aleksandar Rikalo
Details
Add mips N32 ABI suupport (70.96 KB, patch)
2018-04-05 12:07 UTC, Aleksandar Rikalo
Details
Fix tests (17.88 KB, patch)
2018-04-05 12:11 UTC, Aleksandar Rikalo
Details
Add mips N32 ABI suupport (74.12 KB, patch)
2018-04-18 16:34 UTC, Aleksandar Rikalo
Details
Fix tests (22.69 KB, patch)
2018-04-18 16:35 UTC, Aleksandar Rikalo
Details
Add mips N32 ABI support (72.01 KB, patch)
2018-04-19 14:21 UTC, Aleksandar Rikalo
Details
Fix tests (22.83 KB, patch)
2018-04-19 14:21 UTC, Aleksandar Rikalo
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Crestez Dan Leonard 2015-04-01 15:38:05 UTC
It seems that valgrind upstream has no support for the mips n32 abi. I searched around but I found no applicable patches so I made my own. I got it to the point where it can actually run useful software (the stuff I want to debug). It does still generate lots of false positives.

The N32 abi is very similar to the x32 API from the intel world. It has full 64bit registers but 32bit pointers and ints/longs. The purpose of the ABI is to take full advantage of 64bit CPUs but avoid the increased memory consumption. Address space is obviously limited but this limitation does not matter unless you have a lot of RAM.

It uses 32bit elf headers (ELFCLASS32) but it can be distinguished by the EF_MIPS_ARCH_64R2 flag (which indicates 64bit code). It also has it's own syscall numbers. It is much more similar to the N64 abi than the "o32" abi for mips32 cpus. For example the o32 ABI even names registers differently.

Presumably valgrind support for n32 would look similar to support for the x32 ABI, but that does not exist yet.

I made this work by adding a new "mips64n32" architecture which is compiled as a secondary to mips64. In theory mips32 with the o32 ABI could be the secondary for mips64, but this is not supported yet. Eventually valgrind will need to support either "ternary" architectures (for stuff like x32) or just a list of secondary architectures.

It might even be possible to run n32 code without compiling the valgrind tools again, but that would be very difficult. It seems that valgrind has a compile-time dependency on stuff like the size of the elf header, register sizes (implied in the guest machine type) and syscall numbers.

I posted my current patches against valgrind/vex trunk. I'm using git clones for development but the patches should apply cleanly with either "svn patch" or "patch -p1". I also made my git trees public, they contain the same changes split into many smaller patches. It makes it easier to differentiate between ifdef spam and actual subtle changes: 

	https://github.com/cdleonard/valgrind/commits/mips64n32
	https://github.com/cdleonard/valgrind-vex/commits/mips64n32

The diff is not very large. I was able to avoid most copy/pasting by making VG{A,P}_mips64n32 mostly equivalent to VG{A,P}_mips64. This looks tedious but is similar to how ppc64le is implemented.

I would like to push this upstream eventually. Let me know what you think of the approach and what is required in order to get this commited. I did not run the valgrind unit tests because I am cross-compiling. I can try to run them inside a debian-mips VM if it helps.

Reproducible: Always
Comment 1 Crestez Dan Leonard 2015-04-01 15:38:53 UTC
Created attachment 91848 [details]
mips64n32-valgrind.diff
Comment 2 Crestez Dan Leonard 2015-04-01 15:39:13 UTC
Created attachment 91849 [details]
mips64n32-valgrind-vex.diff
Comment 3 Petar Jovanovic 2015-04-02 01:07:31 UTC
(In reply to Crestez Dan Leonard from comment #0)
> It seems that valgrind upstream has no support for the mips n32 abi. I
> searched around but I found no applicable patches so I made my own.

First of all, thanks for the interest and working on this.
Support for n32 ABI has come up a few times, and while it should not be a problem to add it, it is always a challenge to maintain it in an already complex existing MIPS32 vs MIPS64, R1 vs R2, LE vs BE, HF vs SF, FP32 vs FP64 MIPS matrix, plus DSP ASE and future stuff such as MSA or R6 variants. In short, adding new ABI should be considered with precautions.

> Let me know what you think of the approach and what is required in order to get this commited. I did not run the valgrind unit tests because I am cross-compiling. I can try to run them inside a debian-mips VM if it helps.

Making sure your changes:
1.	do not introduce any regressions on any supported MIPS variant
2.	the list of regtest failures for n32 is similar in number to other MIPS supported variants in Valgrind (which is dozen failures or less)

should be really a first step.
Comment 4 Crestez Dan Leonard 2015-04-07 19:15:36 UTC
Created attachment 91934 [details]
mips64n32-V2-valgrind.diff
Comment 5 Crestez Dan Leonard 2015-04-07 19:40:59 UTC
I attached a new version of the valgrind patch. It fixes the hardwired redirections for ld.so and an ifdef mismatch:

-+#if !defined(VGA_mips32) && !defined(VGA_mips64) || !defined(VGA_mips64n32)
++#if !defined(VGA_mips32) && !defined(VGA_mips64) && !defined(VGA_mips64n32)

I did run regression tests on my target system and checked that the n32 does not introduce any regressions for n64. There was an actual regression failure the first time, this is how I found the ifdef mess above.

Because I ran directly on the target I needed to fix sed filter to work with busybox: https://github.com/cdleonard/valgrind/commit/f7f8c88aca93d560691be3e47ffa49f260dc6307 . It's not clear if this kind of patches are welcome.

I also have a bunch of unrelated octeon patches. The mips64n32 patch I posted should be 100% free of cavium-specific noise.

I also tried to run inside a debian-mips chroot but failed with a large number of false positives in memcheck and co. I suspect some issue with debian symbol for mips n64 libc? It's not clear testing this is particularly useful.

I did not run tests for n32 itself. In order to do this I need to hack configure.ac to support some sort of --enable-only32bit for n32. This would cause all the test programs (include non-arch-specific stuff) to compile with -mabi=n32. It would be an useful exercise.

Unfortunately it seems to me that the platform selection for mips is sort of a mess. IMHO manually specifying stuff like -march -mabi with CFLAGS should never be required in a native build. Instead valgrind should explicitly specify the intended -mabi every time. This would produce clearer results working around stuff like gcc defaulting to different ABIs. Does this make sense?

I also read up on https://bugs.kde.org/show_bug.cgi?id=331282 . In order to prevent my patches from interfering with mips o32 support I will keep --host=mips32* (for o32) and --host=mips64* (for n32/n64) separate inside configure.ac
Comment 6 Crestez Dan Leonard 2015-04-09 15:28:27 UTC
It actually seems that gcc does not accept multiple -mabi=* arguments on the command line. It only happened to work for me because my compiler defaults to N64 (which is not true on debian).

So the current tricks in README.mips need to be cleaned up and valgrind needs to pass the correct -mabi argument itself.
Comment 7 Crestez Dan Leonard 2015-04-15 00:40:21 UTC
Created attachment 92034 [details]
mips64n32-V3-valgrind.diff

Attached V3. This was rebased on svn trunk r15091. This is after the tilegx support was integrated so there are a lot of small mostly irrelevant ifdef changes.

Now valgrind will always pass the appropriate -mabi flag on mips by itself as discussed above.

I also made the tests run with the N32 abi. You need to compile with --enable-only32bit to run tests this way. There are significantly more failures than with the default n64 abi. Some of them were fixed by simply using 64bit "long long" instead of "long" in the mips64 test code.
Comment 8 Dejan Jevtic 2015-04-15 10:50:25 UTC
(In reply to Crestez Dan Leonard from comment #7)
I took a quick look at patch and here's my suggestions that maybe can help you.
First of all I see that the most part of patch are changes related to:
+defined(VGA_mips64) || defined(VGA_mips64n32)
or
+#if defined(VGP_mips64_linux) || defined(VGP_mips64n32_linux)

I think that we should avoid adding new VGA_mips64n32 or VGP_mips64n32_linux to the existing code,
and we should use existing VGP_mips64_linux.
What I suggest is that we should check during the compile time if we
are compiling Valgrind for n32 or n64. You can see that in guest_mips_toIR.c
we have #if (__mips_fpr==64) for floating point mode (32 or 64) on mip32.
You should check if compiler is emitting something similar for n32 and use that
to differentiate n32 and n64 ABI on mips64.
Comment 9 Crestez Dan Leonard 2015-04-15 13:49:24 UTC
It would be nice to use VGA_mips64_linux instead of VGA_mips64n32.

Right now the VGA/VGP flags are constructed based on VGCONF_ARCH/VGCONF_OS. I would need a special case for mips where the VGA flag is constructed differently. This seems a bit hacky but I'll try.

It might even be possible to use the same VGCONF_ARCH and only support either N32 or N64 based on a configure-time switch. But being able to run both kinds of binaries seems useful.
Comment 10 Julian Seward 2015-04-15 13:59:35 UTC
[arriving late at the party, as usual]

If mips64n32 is just a 32 bit ABI on a 64 bit CPU, then using the VGA_
classification definitely isn't right.  VGA_ is used to specify the
processor architecture and nothing else, neither the OS nor the ABI.

VGO_ is used to specify the OS.  But that's still Linux.  And the VGP_
classification specifies an (Arch, OS) pairing (known as a Platform,
hence the P).  So at first glance it seems as if VGP_mips64_linux is
the closest to being correct.

However, there is a mechanism for denoting minor variants of a given
platform: the platform-variant (VGPV_) mechanism.  That might be your
best bet.  It was originally developed so that Android variants of
Linux could be cleanly supported.  It takes a third tag, which
specifies the variant.  For example, the "vanilla" arm linux,
that is, VGP_arm_linux, has the variant VGPV_arm_linux_android
when we need android specific variations.

The configure system will (in this example) ensure that VGP_arm_linux
is defined whenever any variant, eg, VGPV_arm_linux_android, is.  Have
a grep around the tree to find examples of the uses of
VGPV_arm_linux_android.

So .. I think you should try to make this a variant of
VGP_mips64_linux, that is, VGPV_mips64_linux_64n32.
Comment 11 Crestez Dan Leonard 2015-04-15 14:42:40 UTC
Right now the names for most of the binaries are formatted based on something like "memcheck_@VGCONF_ARCH_PRI@_@VGCONF_OS@_SOURCES" and duplicated for VGCONF_ARCH_SEC. This does not include VGCONF_PLATVARIANT.

This means that in order to support two ABIs from a single valgrind build a different VGCONF_ARCH needs to be used. It's either that or rewrite a significant portion of the build files.

Only supporting one ABI at configure time would actually be acceptable for me because I compile valgrind separately for boards that mostly run n32 software and boards that mostly run n64 software. It seems like a really nasty limitation to build into valgrind itself. To be fair, dual ABI support is achieved by slightly abusing VGCONF_ARCH.

It's not at all cleat that VGCONF_PLATVARIANT is appropriate. Maybe some day we will have android running on mips64 CPUs with the n32 ABI.

I don't even actually need a VG* define to differentiate between ABIs, I'm sure that I can find something built into gcc.

Ideally the solution would also be applicable to a valgrind port to the x32 ABI. Basically each tool/lib should be built in N different variants.
Comment 12 Crestez Dan Leonard 2015-04-20 16:22:56 UTC
I've been thinking about this and I think that I should drop support for multiple ABIs. This currently works but:
 - It's strongly relies on adding a new VGCONF_ARCH and lots of spammy changes.
 - It abuses the primary/secondary arch system.
 - It requires valgrind to pass the -mabi argument by itself instead of the user passing CFLAGS=*.

I actually think that requiring the user to pass -mabi in CFLAGS is a very bad thing. The fact that valgrind does not simply "./configure && make && make install" on debian mips should be considered a bug. But cleaning this up is outside the scope of N32 support.

It would be easiest to use a gcc built-in macro like _ABIN32.

It would be to define a set of macros like VGABI_32, VGABI_64, VGABI_N32. Initially these would be defined using AC_COMPILE_IFELSE checking the -mabi of the compiler and CFLAGS. This is the only way to do it if valgrind doesn't get to decide on the -mabi to use.

Support for multiple ABIs in the same build can be added separately. The proper way to do this would require build system changes that would also apply to support for the intel x32 abi.

Checks based on VGABI_* would keep the C code clean and obvious. I think that overriding PLATVARIANT for this would be unhelpful. Not only is something like android with mips/n32 reasonable but a single system can have a mix of n32 and n64 binaries.

Let me know if you think this is reasonable (or propose another solution). I'll wait for some sort of consensus before starting to make changes.
Comment 13 Crestez Dan Leonard 2015-04-23 10:20:20 UTC
Created attachment 92180 [details]
mini-mips64n32-valgrind-v1.patch
Comment 14 Crestez Dan Leonard 2015-04-23 10:20:47 UTC
Created attachment 92181 [details]
mini-mips64n32-valgrind-vex-v1.patch
Comment 15 Crestez Dan Leonard 2015-04-23 10:22:05 UTC
Created attachment 92182 [details]
test results (partial)
Comment 16 Crestez Dan Leonard 2015-04-23 10:32:22 UTC
Here's a new version. It no longer supports multiple ABIs at the same time because that required abusing VGCONF_ARCH. The C changes are smaller and cleaner and support for multiple ABIs can be added through autotool hackery anyway.

In order to build for n32 you need to specify CFLAGS="-mabi=n32" explicitly. This is consistent with current practice for mips.

There are a lot of test failures. Many seem to be because backtracing ends early for some reason.

Some of the changes are questionable (assertion relaxations). They are required because registers are 64bit while ints, longs and pointers are all 32bit. This is mips64 with VEX_HOST_WORDSIZE == 4. It's not clear if other similar platforms are supported by valgrind.
Comment 17 Crestez Dan Leonard 2015-04-23 15:56:54 UTC
Created attachment 92185 [details]
mini-mips64n32-valgrind-v2.patch
Comment 18 Crestez Dan Leonard 2015-04-23 15:58:01 UTC
Created attachment 92186 [details]
test results v2
Comment 19 Crestez Dan Leonard 2015-04-23 16:54:17 UTC
New version fixes stacktrace extraction. Many more tests pass now.
Comment 20 Crestez Dan Leonard 2015-04-23 19:48:14 UTC
Created attachment 92187 [details]
valgrind-cavium-mini-mips64n32-v3.log
Comment 21 Crestez Dan Leonard 2015-04-23 19:50:26 UTC
Created attachment 92188 [details]
mini-mips64n32-valgrind-v3.patch
Comment 22 Crestez Dan Leonard 2015-04-23 19:56:36 UTC
With V3 there are many fewer failures (~30). I think the failures in none/tests/scripts/ are because my /bin/sh is actually compiled for n64.

One of the dubious changes I made is change all syscall args to 64bit (instead of word-sized) on mips64 n32. This seems to be required for stuff like lseek to work properly on large files. There is no llseek syscall for the n32 abi. Instead the regular lseek is used and it takes and returns 64bit arguments.

I tested lseek behavior with a separate program, I don't know if the valgrind regression suite has a test for this.
Comment 23 Crestez Dan Leonard 2015-07-20 14:25:00 UTC
Created attachment 93668 [details]
mini-mips64n32-valgrind-v4.patch

Attached mini-mips64n32-valgrind-v4.patch. This is the same as V3 but rebased on top of latest svn trunk@15419. This solves some conflicts with recent SysRes cleanups for mips. The VEX changes are trivial and don't need an update.
Comment 24 Crestez Dan Leonard 2015-11-26 21:48:00 UTC
Created attachment 95761 [details]
valgrind mips64n32 support on top of 3.11.0
Comment 25 Crestez Dan Leonard 2015-11-26 21:49:14 UTC
Created attachment 95762 [details]
valgrind vex mips64n32 support on top of 3.11.0
Comment 26 Crestez Dan Leonard 2015-11-26 21:49:41 UTC
I attached new versions. These should work on top of release 3.11.0.
Comment 27 Maran Pakkirisamy 2017-07-27 08:41:10 UTC
Created attachment 106892 [details]
n32 support valgrind patch rebased on 3.13

This is rebase of the patch in #comment 24 on 3.13 branch.
Applies on trunk rr16461 as well.
Comment 28 Maran Pakkirisamy 2017-07-27 08:43:41 UTC
Created attachment 106893 [details]
n32 support VEX patch rebased on 3.13

This is rebase of the patch in #comment 25 on 3.13 branch.
Applies on VEX trunk r3398 as well.
Comment 29 Maran Pakkirisamy 2017-07-27 08:46:29 UTC
Created attachment 106894 [details]
fix name mismatch

Fixes a minor issue in the N32 support patches.
The same name mismatch issue is found for VGP_ppc32_linux, VGP_ppc64be_linux, VGP_ppc64le_linux as well.
Comment 30 Maran Pakkirisamy 2017-07-27 08:49:22 UTC
Created attachment 106895 [details]
fix building valgrind for N32

These changes were required on top of N32 support patches to build V for N32 successfully.
Comment 31 Aleksandar Rikalo 2018-04-05 12:04:40 UTC
Created attachment 111844 [details]
Introduce RegWord type

On all other architectures size of long matches register width. On mips n32 size of long is 32 bits and register width is 64 bits.
Valgrind is written with assumption that long size matches register width. This is reason why both UWord for valgrind and HWord for VEX match size of long. Long size differs from register size on mips n32 ABI.

This causes problems in several cases:
- For do_syscall arguments is used UWord type. This may cause that 64-bit values lost 32 higher bits.
- In several places where Valgrind needs to check size of registers, using long based types, would give incorrect value.

There are two possible solutions for these problems:
- Change size of HWord or UWord for mips n32 abi- Problem with this solution is that they are already used on great number of places where it is assumed that they match long size. Changing their size would require changes in great number of places, and require new type which matches size of long. Change of this proportion would potentially introduce a great deal of new bugs.
- Introduce a new type which will match size of registers on all platforms and use it where necessary, which is implemented in provided patch.

This patch has no effect on Valgrind's behavior.
Comment 32 Aleksandar Rikalo 2018-04-05 12:07:20 UTC
Created attachment 111845 [details]
Add mips N32 ABI suupport
Comment 33 Aleksandar Rikalo 2018-04-05 12:11:59 UTC
Created attachment 111846 [details]
Fix tests

With the last three changes, there are no failing tests on mips N32.
Comment 34 Aleksandar Rikalo 2018-04-18 16:34:40 UTC
Created attachment 112103 [details]
Add mips N32 ABI suupport

Re-based & arranged patch.
Comment 35 Aleksandar Rikalo 2018-04-18 16:35:34 UTC
Created attachment 112104 [details]
Fix tests

Re-based & arranged patch.
Comment 36 Aleksandar Rikalo 2018-04-19 14:21:22 UTC
Created attachment 112116 [details]
Add mips N32 ABI support
Comment 37 Aleksandar Rikalo 2018-04-19 14:21:53 UTC
Created attachment 112117 [details]
Fix tests
Comment 38 Petar Jovanovic 2018-06-14 18:38:21 UTC
Support for N32 ABI has finally landed in a series of 4 patches:

#1
additional use of RegWord
https://sourceware.org/git/?p=valgrind.git;a=commit;h=8b2fe98aca7568a821c921ba3183f7d62b35a12c

#2
mips64: use RegWord where appplicable
https://sourceware.org/git/?p=valgrind.git;a=commit;h=ac58a6b8578697bdbfdfdcc8d2c70f8f93eef8c0

#3
mips64: add N32 ABI support
https://sourceware.org/git/?p=valgrind.git;a=commit;h=9a6cf7a41c7e7a32e0d0a369b4519b97e2569d3d

#4
mips64: update tests for N32 ABI
https://sourceware.org/git/?p=valgrind.git;a=commit;h=58c1c98db4eb8a66ddea228ab00c8ac1bd7e027c

Please test it now on your machines.

Thanks for the contribution.