Control: tags 905834 + patch moreinfo On Thu, 27 Sep 2018 at 11:10:19 +0200, John Paul Adrian Glaubitz wrote: > Please always CC [email protected] in the future.
Sorry, I thought I had - I must have missed it for that particular bug. > Anyway, attaching an updated patch for m68k. Is this in the form you would send it upstream? I'd prefer it in the form of the individual patches you'd send upstream, rather than one big patch per architecture - that'll mean you don't have to prepare two different versions, and I don't have to discard the whole thing when part of it doesn't apply cleanly. Multiple patches on an email are fine, or so is a merge request on https://salsa.debian.org/gnome-team/mozjs60 if that's easier for you. Is <https://bugzilla.mozilla.org/show_bug.cgi?id=1325771> the right upstream bug reference for all this? Does it work, and do the tests pass? In mozjs52, you said something failed with message "SSN_pattern - 8 = 8 expected: NaN", but I'm not sure which one: presumably js/src/tests/non262/RegExp/RegExp_object.js? Does that one still fail? In mozjs52 I ignored the test result completely for some architectures, but in mozjs60 I've switched to ignoring individual test failures, so that we can still distinguish between "mostly works" and "completely broken" even in the presence of isolated architecture-specific failures (which has already saved us from shipping broken s390x binaries that fail 80% of their tests). If a small number of tests fail (indicating that there are minor architecture-specific bugs, but the library generally works) please patch either js/src/tests/jstests.list or the first line of the tests themselves to mark the test as "skip-if" or "random-if" on m68k, similar to debian/patches/tests-Expect-a-test-to-fail-on-armel.patch and debian/patches/tests-Expect-some-floating-point-tests-to-fail-on-i386.patch; or let me know which ones are failing and I'll add appropriate annotations. > -} JS_HAZ_GC_THING; > +} JS_HAZ_GC_THING __attribute__ ((aligned(4))); This is in architecture-independent code, but seems harmless. I see you had some disputes over how best to fix alignments on <https://bugzilla.mozilla.org/show_bug.cgi?id=1325771>, but in Debian we only care about gcc (and maybe clang) so diverging from what you upstreamed by using this gcc-specific implementation seems fine. > +static constexpr size_t sizemax(size_t a, size_t b) > +{ > + return (a > b) ? a : b; > +} > + > INSTANTIATE(int, int, prim1, 2 * sizeof(int)); > -INSTANTIATE(int, long, prim2, 2 * sizeof(long)); > +INSTANTIATE(int, long, prim2, sizeof(long) + sizemax(sizeof(int), > alignof(long))); > > struct EmptyClass { explicit EmptyClass(int) {} }; > struct NonEmpty { char mC; explicit NonEmpty(int) {} }; > > INSTANTIATE(int, EmptyClass, both1, sizeof(int)); > -INSTANTIATE(int, NonEmpty, both2, 2 * sizeof(int)); > +INSTANTIATE(int, NonEmpty, both2, sizeof(int) + alignof(int)); > INSTANTIATE(EmptyClass, NonEmpty, both3, 1); https://bug1325771.bmoattachments.org/attachment.cgi?id=8831556 makes it a lot clearer why this is correct, so I'd really prefer to apply the individual patches with their justifications intact. Thanks, smcv
