On Tue, 14 Jan 2020 17:25:58 +0100
Jeremie Courreges-Anglas wrote:
> 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.
Given your more significant work on it compared to mine, feel free to
commit it :)
>
> > +
> > +.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
Sweet!
> ok?
It stills works fine on macppc. OK cwen@ but...
> 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
^ This causes the following on powerpc:
quickjs-2019.10.27p0(lang/quickjs):
Bogus WANTLIB: atomic.3 (/usr/local/bin/qjsc) (NOT REACHABLE)
This is happening because "COMPILER_LANGS=c++" pulls gcc-libs.
As discussed elsewhere with jca@ this line should go for practical
reasons.
jca@ pointed out that a LIB_DEPENDS on lang/gcc/8,-libs would be
required to allow that line on macppc, and actually a bunch of other
ports already only use C but depend on the default COMPILER_LANGS value
to get to libatomic on powerpc/hppa.
> 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
>
