1526653 - Spidermonkey building error on Raspberry pi 3

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P3)

Description

[dhsodhao52]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/71.0.3578.98 Chrome/71.0.3578.98 Safari/537.36

Steps to reproduce:

I'm trying to build spidermonkey on the raspberry pi 3, but I have encountered the error by following these instructions.

1. get the source cord and prerequisites from the mozilla home (https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey) using 'git'.
2. sequentially did autoconf, configure and make.

in my raspberry pi, I installed these versions.
cargo 1.32.0
rustc 1.32.0
gcc 6.3.0
clang 7.0.0

Actual results:

make[3]: Entering directory '/home/pi/development/spidermonkey/js/src/build_OPT.OBJ/js/src/wasm'
/usr/local/clang_7.0.0/bin/clang++ -o Unified_cpp_js_src_wasm2.o -c -I/home/pi/development/spidermonkey/js/src/build_OPT.OBJ/dist/system_wrappers -include /home/pi/development/spidermonkey/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DENABLE_BINARYDATA -DENABLE_WASM_BULKMEM_OPS -DENABLE_WASM_REFTYPES -DENABLE_WASM_GENERALIZED_TABLES -DENABLE_WASM_GC -DWASM_PRIVATE_REFTYPES -DJS_CACHEIR_SPEW -DJS_STRUCTURED_SPEW -DEXPORT_JS_API -DMOZ_HAS_MOZGLUE -I/home/pi/development/spidermonkey/js/src/wasm -I/home/pi/development/spidermonkey/js/src/build_OPT.OBJ/js/src/wasm -I/home/pi/development/spidermonkey/js/src/build_OPT.OBJ/js/src -I/home/pi/development/spidermonkey/js/src -I/home/pi/development/spidermonkey/js/src/build_OPT.OBJ/dist/include -fPIC -DMOZILLA_CLIENT -include /home/pi/development/spidermonkey/js/src/build_OPT.OBJ/js/src/js-confdefs.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-noexcept-type -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-sized-deallocation -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-rtti -fno-exceptions -fno-math-errno -pthread -pipe -g -O3 -fno-omit-frame-pointer -funwind-tables -fno-strict-aliasing -Werror=format -Wno-shadow -MD -MP -MF .deps/Unified_cpp_js_src_wasm2.o.pp /home/pi/development/spidermonkey/js/src/build_OPT.OBJ/js/src/wasm/Unified_cpp_js_src_wasm2.cpp
In file included from /home/pi/development/spidermonkey/js/src/build_OPT.OBJ/js/src/wasm/Unified_cpp_js_src_wasm2.cpp:29:
/home/pi/development/spidermonkey/js/src/wasm/WasmSignalHandlers.cpp:505:19: error: field has incomplete
type 'struct user_vfp'
struct user_vfp ufp;
^
/home/pi/development/spidermonkey/js/src/wasm/WasmSignalHandlers.cpp:505:10: note: forward declaration of
'user_vfp'
struct user_vfp ufp;
^
/home/pi/development/spidermonkey/js/src/wasm/WasmSignalHandlers.cpp:506:23: error: field has incomplete
type 'struct user_vfp_exc'
struct user_vfp_exc ufp_exc;
^
/home/pi/development/spidermonkey/js/src/wasm/WasmSignalHandlers.cpp:506:10: note: forward declaration of
'user_vfp_exc'
struct user_vfp_exc ufp_exc;
^
2 errors generated.
/home/pi/development/spidermonkey/config/rules.mk:812: recipe for target 'Unified_cpp_js_src_wasm2.o' failed
make[3]: *** [Unified_cpp_js_src_wasm2.o] Error 1
make[3]: Leaving directory '/home/pi/development/spidermonkey/js/src/build_OPT.OBJ/js/src/wasm'
/home/pi/development/spidermonkey/config/recurse.mk:74: recipe for target 'js/src/wasm/target' failed
make[2]: *** [js/src/wasm/target] Error 2
make[2]: Leaving directory '/home/pi/development/spidermonkey/js/src/build_OPT.OBJ'
/home/pi/development/spidermonkey/config/recurse.mk:32: recipe for target 'compile' failed
make[1]: *** [compile] Error 2
make[1]: Leaving directory '/home/pi/development/spidermonkey/js/src/build_OPT.OBJ'
/home/pi/development/spidermonkey/config/rules.mk:400: recipe for target 'default' failed
make: *** [default] Error 2

Comment 1

[Lars T Hansen [:lth]]

That's annoying, sorry about that.

user_vfp and user_vfp_exc should have been defined in sys/user.h, as they are on Android and vanilla Linux, but it's possible your OS is different.

Are you running the Raspbian OS or something else?

You can work around this locally as follows:

In js/src/wasm/WasmSignalHandler.cpp, there are two #ifdef blocks that are guarded like this:

#if defined(__linux__) && defined(__arm__)

You can change them to this:

#if 0 && defined(__linux__) && defined(__arm__)

The only consequence you should experience is that if you're trying to run WebAssembly content that uses unaligned floating-point accesses then those accesses may be reported as memory bounds error. It's extremely unlikely that this will be a problem for you.

Comment 2

[Lars T Hansen [:lth]]

I'd actually be interested in seeing what your /usr/include/sys/user.h looks like. I don't have an RPi3 myself and the only source I've found on the net so far seems to have a user.h that pertains to x86 class systems, so it's probably not the one I want.

Comment 3

[dhsodhao52]

I am working on the Raspian currently. However, I cannot find /usr/include/sys/user.h file on RPi3, since there are no directory named 'sys' in include. Only 'user.h' file I have on library is at arm-linux-gnueabihf.
And it looks like below.

/* Copyright (C) 1998-2016 Free Software Foundation, Inc.
   This file is part of the GNU C Library.

   The GNU C Library is free software; you can redistribute it and/or
   modify it under the terms of the GNU Lesser General Public
   License as published by the Free Software Foundation; either
   version 2.1 of the License, or (at your option) any later version.

   The GNU C Library is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
   Lesser General Public License for more details.

   You should have received a copy of the GNU Lesser General Public
   License along with the GNU C Library.  If not, see
   <http://www.gnu.org/licenses/>.  */

#ifndef _SYS_USER_H
#define _SYS_USER_H	1

/* The whole purpose of this file is for GDB and GDB only.  Don't read
   too much into it.  Don't use it for anything other than GDB unless
   you know what you are doing.  */

struct user_fpregs
{
  struct fp_reg
  {
    unsigned int sign1:1;
    unsigned int unused:15;
    unsigned int sign2:1;
    unsigned int exponent:14;
    unsigned int j:1;
    unsigned int mantissa1:31;
    unsigned int mantissa0:32;
  } fpregs[8];
  unsigned int fpsr:32;
  unsigned int fpcr:32;
  unsigned char ftype[8];
  unsigned int init_flag;
};

struct user_regs
{
  unsigned long int uregs[18];
};

struct user
{
  struct user_regs regs;	/* General registers */
  int u_fpvalid;		/* True if math co-processor being used. */

  unsigned long int u_tsize;	/* Text segment size (pages). */
  unsigned long int u_dsize;	/* Data segment size (pages). */
  unsigned long int u_ssize;	/* Stack segment size (pages). */

  unsigned long start_code;	/* Starting virtual address of text. */
  unsigned long start_stack;	/* Starting virtual address of stack. */

  long int signal;     		/* Signal that caused the core dump. */
  int reserved;			/* No longer used */
  struct user_regs *u_ar0;	/* help gdb to find the general registers. */

  unsigned long magic;		/* uniquely identify a core file */
  char u_comm[32];		/* User command that was responsible */
  int u_debugreg[8];
  struct user_fpregs u_fp;	/* Floating point registers */
  struct user_fpregs *u_fp0;	/* help gdb to find the FP registers. */
};

#endif  /* sys/user.h */

By the way, the build has been done successfully! Thanks for your help.

Comment 4

[Lars T Hansen [:lth]]

Thanks. That's as I feared -- it's the only user.h I found in the Raspbian sources also, I was hoping the device would have something better. The user_fpregs structure is completely inappropriate for ARM.

Ideally we'd be able to #ifdef our way out of this but there doesn't seem to be a reliable #ifdef for Raspbian. I may resort to just placing some comments in the code so that the next person to attempt this will be less puzzled.

Comment 5

[Lars T Hansen [:lth]]

Attachment: Bug 1526653 - Document some ARM Linux compile problems. r?luke

Not every ARM Linux distro (all of them tier-3 apart from Android) has
a sys/user.h that provides the necessary structures for us to emulate
unaligned FP accesses. Raspbian is a case in point; the only user.h
on the system is some generic glibc thing that is completely wrong for
ARM.

We can't really hope to #ifdef our way out of this -- for example,
there does not seem to be a reliable preprocessor name for Raspbian,
nor is there a canonical preprocessor name that sys/user.h exports
when the desired structures are defined.

Therefore, add some comments to explain the problem and introduce a
choke point that tier-3 porters can use if they run into the problem.

Comment 6

[dhsodhao52]

Thank you for your help!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/322de2cc8019
Document some ARM Linux compile problems.  r=luke

Comment 8

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/322de2cc8019

Comment 9

[tt_1]

I'm afraid that didn't fully work. Compile with the patch from 322de2cc8019:

cat build.log | grep error: -b3

1:06.93 In file included from /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-66.0-r1/work/firefox-66.0/ff/js/src/wasm/Unified_cpp_js_src_wasm2.cpp:29:
1:06.93 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-66.0-r1/work/firefox-66.0/js/src/wasm/WasmSignalHandlers.cpp:538:19: error: field 'ufp' has incomplete type 'user_vfp'
1:06.93 struct user_vfp ufp;
1:06.93 ^~~
1:06.93 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-66.0-r1/work/firefox-66.0/js/src/wasm/WasmSignalHandlers.cpp:538:10: note: forward declaration of 'struct user_vfp'
1:06.93 struct user_vfp ufp;
1:06.93 ^~~~~~~~
1:06.94 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-66.0-r1/work/firefox-66.0/js/src/wasm/WasmSignalHandlers.cpp:539:23: error: field 'ufp_exc' has incomplete type 'user_vfp_exc'
1:06.94 struct user_vfp_exc ufp_exc;
1:06.94 ^~~~~~~
1:06.94 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-66.0-r1/work/firefox-66.0/js/src/wasm/WasmSignalHandlers.cpp:539:10: note: forward declaration of 'struct us

Comment 10

[Lars T Hansen [:lth]]

The patch did not fix anything at all - it only created a point for you so that you could go in and fix it yourself easily if you run into this problem. With the patch applied (or using current sources), you should comment out the line with the #define on it, the one identified as 1.27 if you click on the patch link above.

Comment 11

[tt_1]

Then why is this bug closed as fixed? It's about the compile failure, and not the fact that this is undocumented.

Comment 12

[Lars T Hansen [:lth]]

Note the remark in comment 5 that there isn't a fix for this because there isn't a reliable preprocessor name that allows us to recognize when the code is being compiled for the Raspberry Pi 3. The "fix" is therefore that when you get the compile error there is a prominent comment in the code that tells you how to work around it.

ARM Linux, including the RPi3, is a tier-3 platform for us and there will therefore be some things that might not work out of the box. This is one of those things; it would be nice if that were not the case, but it's life.

Comment 13

[tt_1]

Ok, to summ it up:

spidermonkey includes user.h on armv6/7. Only Android serves that specific header with their kernel-headers. Glibc has a header with the same name, but it doesn't offer the same functions, therefore compiling on arm + linux ends up pulling in a wrong header from glibc. The fix forces distros to patch indivially, depending on arch.

Wouldn't it be a much better idea to check for the header with autoconf during configure?

Or to put all of the code in question behind another

#if defined(arm) && defined(ANDROID)

?

Comment 14

[Lars T Hansen [:lth]]

Again, ARM Linux is not a well supported target. Patches are welcome, but disabling this functionality is basically not the right thing, as it leads to broken wasm programs, so in a sense it's better that there's a compilation failure + local workaround than a quiet failure.

Do you have access to other ARM Linux hardware / distros than the RPi3? I'm curious if this is a Raspbian issue or if it's endemic, as you seem to think.

Comment 15

[tt_1]

Yes, I do have access to other hardware and distro. Its an rpi2, it runs Gentoo with CHOST=armv7a-unknown-linux-gnueabihf.

According to my research, including user.h is clearly an androidism, it is simply unheard of for gnu based linux-headers. Glibc headers user.h seems to aim for debugging purpose and gives even a warning to not include it on other conditions, unless you know what you're doing. Clearly this must fail.

Comment 16

[Lars T Hansen [:lth]]

Well, that's annoying :-/

As I wrote briefly in comment 14, it's really not the right thing to quietly hide this problem on ARM Linux, as it will make some programs (probably not many, but some) fail quietly.

We can clearly #ifdef the inclusion of user.h on ANDROID. But for ARM Linux we'd want to instead provide the information that is in user.h directly, as a fallback. The magic-number handshake with the kernel data structure allows us to detect when we're looking at the correct data so it feels relatively safe, but we'll need to test that. It also looks like I can just take the necessary definitions from the header on Android (it has a compatible license), though I'll need to check that with the proper authorities.

If I make a patch, are you willing to help me troubleshoot it?

Comment 17

[tt_1]

Absolutly, I can offer to apply any patch on top of firefox-66.0.2, to compile it with gcc and to runtime test it. Also I've got a working cross-compile setup, so results will be there within a reasonable amount of time. UTC+1 here, if you need to know! (:

Comment 18

[Lars T Hansen [:lth]]

OK. Won't be today but I'll try to get to it within a reasonable amount of time. I'm in UTC+1 myself so turnaround should be swift once we get going.

Comment 20

[Ray Vine]

Cross compiling FF66 for RPi3, I had a similar error.

As per comment #10, applying the patch from 322de2cc8019 and commenting out
//# define WASM_EMULATE_ARM_UNALIGNED_FP_ACCESS
allowed the build to complete.

---

But for ARM Linux we'd want to instead provide the information that is in user.h directly, as a fallback.
...
It also looks like I can just take the necessary definitions from the header on Android …

I tried that and it works.
Replaced # include <sys/user.h> with the definitions from arch/arm/include/asm/user.h from the kernel source which has a GPL-2.0 licence:

--- js/src/wasm/WasmSignalHandlers.cpp
+++ js/src/wasm/WasmSignalHandlers.cpp
@@ -232 +232,18 @@
-#  include <sys/user.h>
+/*
+ * User specific VFP registers. If only VFPv2 is present, registers 16 to 31
+ * are ignored by the ptrace system call and the signal handler.
+ */
+struct user_vfp {
+	unsigned long long fpregs[32];
+	unsigned long fpscr;
+};
+
+/*
+ * VFP exception registers exposed to user space during signal delivery.
+ * Fields not relavant to the current VFP architecture are ignored.
+ */
+struct user_vfp_exc {
+	unsigned long	fpexc;
+	unsigned long	fpinst;
+	unsigned long	fpinst2;
+};

.

Comment 21

[tt_1]

@lth: any input from you about the patch from #20?

Comment 23

[Lars T Hansen [:lth]]

(In reply to tt_1 from comment #21)

@lth: any input from you about the patch from #20?

I can't take GPL code (any version), though otherwise the patch seems OK.

I really have very limited time to work on this so it's unlikely I'll do much to get this fixed soon. An acceptable patch might possibly be derived from Android headers and would ideally arrive in its own file with the Android copyright notice intact, and would just be included conditionally from WasmSignalHandlers.cpp.

I'm not discounting that there might be other solutions, but including third-party licensed code into Firefox is subject to a number of restrictions and will have to go through licensing review. Using a license we already accept, such as the Android License (see about:license), simplifies this.

Comment 24

[Mike Hommey [:glandium]]

The Android user.h has the exact same structures defined, in a BSD-licensed header, without the comments. Really, structure definitions are barely subject to copyright, and the patch from comment 20, minus the comments, should be fine. If copyright applied, it could be argued the Android header is derived from the GPL header from the linux kernel, and it can't be relicensed under the BSD anyways, so if including the structs in Firefox would be a licensing problem, so would including the Android headers with dubious licensing. Mike, what do you think?

Comment 25

[Mike Hoye [:mhoye]]

I'll see if I can investigate the provenance of those Android headers, to see if/how we can proceed. That's some suspicious looking code right there, for sure.

Leaving the NI open.

Comment 26

[Mike Hoye [:mhoye]]

So, the code in Android, ostensibly licensed under Android's 2-clause BSD:

https://android.googlesource.com/platform/bionic/+/5390173/libc/include/sys/user.h#210

... looks an awful lot like copypasta from this 11 year old kernel code:

https://github.com/torvalds/linux/blame/249155c20f9b0754bc1b932a33344cfb4e0c2101/arch/arm/include/asm/user.h#L98

...that antedates Android as a whole, and is GPL-2.0 licensed.

I haven't been able to find a common antecedent older than that for those structs that would permit dual-relicensing. Unless there's some underlying convention here that I don't see, "structure definitions are barely subject to copyright" is not at all how I have understood the ongoing OvG case to be going.

I'm deeply reluctant to let this stuff into the tree casually just because we've got this android-shaped fig leaf lying around.

Comment 27

[Mike Hommey [:glandium]]

Taking a step back, there are two things worth noting:

• The vfp_sigframe struct that is currently in mozilla-central, is a copy of the same struct from the Linux kernel, with the same GPL license (and no copypasta in some Android header). If adding user_vfp and user_vfp_exc is problematic, then vfp_sigframe being there is already problematic.
• The code assumes only CONFIG_VFP is enabled in the kernel, and falls flat if any of CONFIG_CRUNCH or CONFIG_IWMMXT is enabled. It should scan the auxiliary signal frame, looking at the magic number and the following size, until it finds an entry with VFP_MAGIC, or stop when encountering a magic number with the value 0 (which corresponding to end_magic in aux_sigframe).

Comment 28

[Lars T Hansen [:lth]]

+luke for visibility, as this is fundamental to performant wasm execution on ARM.

The vfp_sigframe was inserted by me, on the assumption that documented APIs to the kernel are fair game. The intent was obviously to expose the Android kernel data to Firefox so that we could properly emulate failed accesses. Almost certainly the code was taken from / transcribed from the Android kernel sources, but I don't have a record of this, and in any case the Android version is identical to the Linux version. Our transcription of the structure is not quite verbatim (I left off an attribute) but that's just quibbling.

(As for handling CONFIG_CRUNCH and CONFIG_IWMMXT, testing did not indicate that more was needed for the use case than what I did do but reading between the lines here these are signal frames that can contain additional system-specific state, and I suppose only testing on machines having that hardware will reveal problems. I'll keep that in mind once we've resolved the licensing issue.)

Comment 29

[dflogeras2]

This also affects me as Gentoo is moving to the 68 ESR series (I try to follow that with my ARM machine(s)).

I applied the patch listed here:
https://bugs.gentoo.org/681036

which is actually cherry-picked from Debian here:
https://salsa.debian.org/mozilla-team/firefox/commit/fd6847c9416f9eebde636e21d794d25d1be8791d

and got a successful build for armv7a running on a cubieboard2 (sunxi a20).

As I understand it from the above conversation, the Debian patch is basically the proposed patch minus the comments.

Has there been any more motion on getting this integrated?

Comment 30

[Lars T Hansen [:lth]]

I'm going to look at this again. From looking at V8 and the ARM manuals, and considering that it looks like we can always do unaligned integer loads without signal handling, we can have a two-pronged strategy for doing unaligned floating point loads without signal handling: if NEON is available then VLD1 can (apparently) be used to load the value with lenient alignment checking; and if not, we can use an integer load followed by a move-to-floating. Ditto for stores. That would allow us not to include the new kernel structure, and it should allow us to remove the one that's already there.

I don't know how much I relish the prospect of backporting that type of fix to ESR68, but if we're lucky it'll be a small patch.

Comment 31

[dflogeras2]

Thanks Lars, for the update. Definitely beyond my skillset to offer any help making the changes, but I am happy to help test on my hardware. Just ping.

Comment 32

[Mike Hommey [:glandium]]

and considering that it looks like we can always do unaligned integer loads without signal handling

Except when the kernel is configured to not allow unaligned accesses via /proc/cpu/alignment.

Comment 33

[Lars T Hansen [:lth]]

(In reply to Mike Hommey [:glandium] (high latency) from comment #32)

and considering that it looks like we can always do unaligned integer loads without signal handling

Except when the kernel is configured to not allow unaligned accesses via /proc/cpu/alignment.

Yes, I'm aware of that (and what a mess that is), but so far as I know we've yet to encounter any kernel such configured, and I'm willing to make the assumption that we won't (for Firefox). It makes sense to allow that behavior for embedded-type systems, but for consumer systems it's essentially hostile. If we still supported ARMv6 I would be more worried, but we don't. Anyway we can conceivably add a guard that runs on startup that checks the state of that bit. It's even OK if the system is set up to handle unaligned loads by kernel fixups, since in practical terms unaligned accesses will be unusual. If the system is set up to handle unaligned loads by address masking we're hosed anyway and really should just not allow code to run.

My plan of action here is to (a) remove the byte-by-byte loads we currently use for integer loads that are declared as unaligned and (b) remove the signal handling machinery and fall back on VLD1 / LDR+VMOV as described above. I'll squirrel the code away somewhere but I expect we'll never need it.

Comment 34

[Jan de Mooij [:jandem]]

For what it's worth, the Baseline Interpreter assumes unaligned integer loads are fine on all platforms with a JIT backend, including ARM32. V8 also has this assumption last I checked.

Comment 35

[Lars T Hansen [:lth]]

This should now be fixed, after bug 1587757 was fixed.