On Mon, Jan 13 2020, Charlene Wendling <[email protected]> wrote:
> Hi!
>
>> http://build-failures.rhaalovely.net/powerpc/last/lang/quickjs.log
> (not the same issue on sparc64 and mips64)
The issue there is pointer truncation because only amd64 and arm64 are
recognized as 64 bits archs.
> As mentioned by the COMPILER comment, it requires stdatomic so we need
> to link it on powerpc and probably hppa.
>
> Later, i've found out that on big endian archs an extra define seems
> needed, because none of our includes define WORDS_BIGENDIAN, and it
> is not defined by the upstream Makefile.
#undef WORDS_BIGENDIAN is hardcoded in cutils.h, we should just adjust
the header to our needs. Setting -DWORDS_BIGENDIAN on the command-line
isn't enough because of the #undef.
> If you wonder why i searched
> for it, it's because tests are successfully failing:
>
> --8<--
> ===> Regression tests for quickjs-2019.10.27
> cc -O2 -pipe -DWORDS_BIGENDIAN -Wall -MMD -MF .obj/bjson.pic.o.d
> -Wno-array-bounds -D_GNU_SOURCE -DCONFIG_VERSION=\"2019-10-27\" -fPIC
> -DJS_SHARED_LIBRARY -c -o .obj/tests/bjson.pic.o tests/bjson.c
> cc -latomic -shared -o tests/bjson.so .obj/tests/bjson.pic.o -lm
> ./qjs tests/test_closure.js
> ./qjs tests/test_op.js
> ./qjs tests/test_builtin.js
> Error: assertion failed: got
> |0,0,255,255,0,0,0,0,63,128,0,0,255,255,255,255|, expected
> |0,0,255,255,0,0,0,0,0,0,128,63,255,255,255,255|
> at assert (tests/test_builtin.js:17)
> at test_typed_array (tests/test_builtin.js:410)
> at <eval> (tests/test_builtin.js:637)
> -->8--
>
> Tests are using javascript TypedArrays in many places, and it appears
> that these arrays are byte-order dependent [0]. It would require a
> lot of patching, so i think we should pass it, as far as ports are
> concerned. The latest version has no changes that would solve the
> issue.
Yep. I'm not sure how to disable those tests on BE archs so we can get
useful test results. For now I just disabled the whole typed array test
function using sed -i.
> It indeed builds [1] on macppc. No REVISION bump is needed, we have
> no quickjs package built on all ${BE_ARCHS}.
>
> Comments/feedback are welcome,
>
> Charlène.
>
>
> [0]
> https://hacks.mozilla.org/2017/01/typedarray-or-dataview-understanding-byte-order/
> [1] https://bin.charlenew.xyz/quickjs.log
>
>
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/lang/quickjs/Makefile,v
> retrieving revision 1.1.1.1
> diff -u -p -u -p -r1.1.1.1 Makefile
> --- Makefile 21 Dec 2019 14:24:03 -0000 1.1.1.1
> +++ Makefile 13 Jan 2020 16:37:11 -0000
> @@ -23,4 +23,15 @@ COMPILER = base-clang ports-gcc
> USE_GMAKE = Yes
> MAKE_FLAGS = CC="${CC}"
>
> +# Fix "undefined reference to `__atomic_store_8'"
> +.if ${MACHINE_ARCH:Mpowerpc} || ${MACHINE_ARCH:Mhppa}
> +WANTLIB += atomic
> +MAKE_FLAGS += LDFLAGS="${LDFLAGS} -latomic"
> +.endif
This part is ok jca@ if you want to commit it right away.
> +
> +.include <bsd.port.arch.mk>
> +.if ${PROPERTIES:Mbe}
> +CFLAGS += -DWORDS_BIGENDIAN
> +.endif
> +
> .include <bsd.port.mk>
This part isn't effective because of the #undef in cutils.h. It doesn't
seem to affect tests much, though. :)
Here's a diff with a bunch of fixes and tweaks along with your -latomic
fix:
- tweak Makefile to make it look more like Makefile.template
- set COMPILER_LANGS since c++ support isn't needed
- use endian.h to detect endianness
- use __LP64__ to detect 64 bits archs and avoid pointer truncation that
breaks sparc64 and mips64
- use __LP64__ to detect 64 bits archs and properly size bignums
- bump REVISION because arm64 is affected by the latter
ok?
Index: Makefile
===================================================================
RCS file: /cvs/ports/lang/quickjs/Makefile,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 Makefile
--- Makefile 21 Dec 2019 14:24:03 -0000 1.1.1.1
+++ Makefile 14 Jan 2020 15:10:29 -0000
@@ -1,10 +1,12 @@
# $OpenBSD: Makefile,v 1.1.1.1 2019/12/21 14:24:03 bcallah Exp $
-V = 2019.10.27
COMMENT = small, embeddable JavaScript engine in C
+
+V = 2019.10.27
+REVISION = 0
DISTNAME = quickjs-${V:S/./-/g}
PKGNAME = quickjs-${V}
-EXTRACT_SUFX = .tar.xz
+
CATEGORIES = lang
HOMEPAGE = https://bellard.org/quickjs/
@@ -16,11 +18,19 @@ PERMIT_PACKAGE = Yes
WANTLIB += c m
MASTER_SITES = https://bellard.org/quickjs/
+EXTRACT_SUFX = .tar.xz
# Requires stdatomic
COMPILER = base-clang ports-gcc
+COMPILER_LANGS = c
USE_GMAKE = Yes
MAKE_FLAGS = CC="${CC}"
+
+# Fix "undefined reference to `__atomic_store_8'"
+.if ${MACHINE_ARCH:Mpowerpc} || ${MACHINE_ARCH:Mhppa}
+WANTLIB += atomic
+MAKE_FLAGS += LDFLAGS="${LDFLAGS} -latomic"
+.endif
.include <bsd.port.mk>
Index: patches/patch-cutils_h
===================================================================
RCS file: patches/patch-cutils_h
diff -N patches/patch-cutils_h
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-cutils_h 14 Jan 2020 15:10:29 -0000
@@ -0,0 +1,21 @@
+$OpenBSD$
+
+Properly detect endianness.
+
+Index: cutils.h
+--- cutils.h.orig
++++ cutils.h
+@@ -28,8 +28,11 @@
+ #include <stdlib.h>
+ #include <inttypes.h>
+
+-/* set if CPU is big endian */
+-#undef WORDS_BIGENDIAN
++#include <endian.h>
++
++#if BYTE_ORDER == BIG_ENDIAN
++#define WORDS_BIGENDIAN
++#endif
+
+ #define likely(x) __builtin_expect(!!(x), 1)
+ #define unlikely(x) __builtin_expect(!!(x), 0)
Index: patches/patch-libbf_h
===================================================================
RCS file: patches/patch-libbf_h
diff -N patches/patch-libbf_h
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-libbf_h 14 Jan 2020 15:10:29 -0000
@@ -0,0 +1,16 @@
+$OpenBSD$
+
+Use more generic test for 64 bits platform.
+
+Index: libbf.h
+--- libbf.h.orig
++++ libbf.h
+@@ -27,7 +27,7 @@
+ #include <stddef.h>
+ #include <stdint.h>
+
+-#if defined(__x86_64__)
++#if defined(__LP64__)
+ #define LIMB_LOG2_BITS 6
+ #else
+ #define LIMB_LOG2_BITS 5
Index: patches/patch-quickjs_h
===================================================================
RCS file: patches/patch-quickjs_h
diff -N patches/patch-quickjs_h
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-quickjs_h 14 Jan 2020 15:10:29 -0000
@@ -0,0 +1,16 @@
+$OpenBSD$
+
+Use more generic test for 64 bits platform.
+
+Index: quickjs.h
+--- quickjs.h.orig
++++ quickjs.h
+@@ -53,7 +53,7 @@ typedef struct JSClass JSClass;
+ typedef uint32_t JSClassID;
+ typedef uint32_t JSAtom;
+
+-#if defined(__x86_64__) || defined(__aarch64__)
++#if defined(__LP64__)
+ #define JS_PTR64
+ #define JS_PTR64_DEF(a) a
+ #else
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
