Closed Bug 932991 Opened 6 years ago Closed 6 years ago

build broken on OpenBSD/sparc64 after header reordering in bug 898274

Categories

(Core :: JavaScript Engine, defect)

x86_64
OpenBSD
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: gaston, Assigned: gaston)

References

Details

Attachments

(1 file)

I don't get why it surfaces now, but it seems https://hg.mozilla.org/mozilla-central/diff/200fae26b271/js/src/jscpucfg.h broke OpenBSD, see http://buildbot.rhaalovely.net/builders/mozilla-central-sparc64/builds/595

eg++ -o testIntTypesABI.o -c  -fvisibility=hidden -DEXPORT_JS_API -DIMPL_MFBT -DMOZ_GLUE_IN_PROGRAM -DNO_NSPR_10_SUPPORT -I/home/buildslave/mozilla-central-sparc64/build/js/src -I.. -I/home/buildslave/mozilla-central-sparc64/build/js/src/jsapi-tests -I. -I../../../dist/include  -I/data/obj/buildslave/m-c/dist/include/nspr       -fPIC   -DMOZILLA_CLIENT -include ../js-confdefs.h -MD -MP -MF .deps/testIntTypesABI.o.pp  -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-limits -Wempty-body -Werror=conversion-null -Wsign-compare -Wno-invalid-offsetof -fno-rtti -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -pipe  -DNDEBUG -DTRIMMED -g -O -fomit-frame-pointer  /home/buildslave/mozilla-central-sparc64/build/js/src/jsapi-tests/testIntTypesABI.cpp
testJSEvaluateScript.o
In file included from /usr/include/machine/endian.h:7:0,
                 from /home/buildslave/mozilla-central-sparc64/build/js/src/jscpucfg.h:56,
                 from /home/buildslave/mozilla-central-sparc64/build/js/src/jsapi-tests/testIntTypesABI.cpp:12:
/usr/include/sys/endian.h:162:1: error: '__uint64_t' does not name a type
 __uint64_t htobe64(__uint64_t);
 ^
/usr/include/sys/endian.h:163:1: error: '__uint32_t' does not name a type
 __uint32_t htobe32(__uint32_t);
 ^
/usr/include/sys/endian.h:164:1: error: '__uint16_t' does not name a type


I dont know why it surfaces now since a build from two days ago was successful (see http://buildbot.rhaalovely.net/builders/mozilla-central-sparc64/builds/593) .. maybe because i was disabling the helper thread to workaround issue #912168 ?

Will try reverting the offending rev to see if it fixes the build at least.
Depends on: 898274
Flags: needinfo?(n.nethercote)
jscpucfg.h gets included in many places.  It's odd that this compile error would only surface now.  Could there have been some kind of header update on the machine with the breakage?

Reverting the order may break the include ordering check done by config/check_spidermonkey_style.py.  (Run |make check| in js/src/ to see.)  If so, you'll need to add jscpucfg.h to the |included_inclnames_to_ignore| list.  And note that you'll need to modify both config/check_spidermonkey_style.py and js/src/config/check_spidermonkey_style.py.
Flags: needinfo?(n.nethercote)
To be honest, i dont really trust an automated process moving includes around on a NPOTB platform, since it has probably no idea of the needed ordering on the said platform. machine/endian.h is only seen on BSDs (since bug #714312).

And the corresponding buildslave hasnt been updated recently, so no env change between a successful and an ok build.
Reverting the header reordering fixes the build for me.

 /* BSDs */
 #elif defined(JS_HAVE_MACHINE_ENDIAN_H)
+# include <sys/types.h>
 # include <machine/endian.h>
-# include <sys/types.h>
If i do it this way (ie adding jscpucfg.h to included_inclnames_to_ignore) it doesnt seem to take the change into account:

--- check_spider_monkey_style.py expected output
+++ check_spider_monkey_style.py actual output
@@ -1,3 +1,6 @@
+js/src/jscpucfg.h:56:57: error:
+    <sys/types.h> should be included after <machine/endian.h>
+

To me it seems the checking should be relaxed in check_includes_order, not check_includes_statement. And i'm not sure adding <sys/types.h> to oddly_ordered_inclnames is the best solution...
That version works, gmake check-style is happy with it.
Assignee: nobody → landry
Attachment #826705 - Flags: review?(n.nethercote)
Attachment #826705 - Attachment is patch: true
Comment on attachment 826705 [details] [diff] [review]
add machine/endian.h to oddly_ordered_inclnames + partly rever 898274.

Review of attachment 826705 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!  I can't remember if you have commit access;  please mark the checkin-needed flag if not.

::: config/check_spidermonkey_style.py
@@ +94,5 @@
>  oddly_ordered_inclnames = set([
>      'ctypes/typedefs.h',        # Included multiple times in the body of ctypes/CTypes.h
>      'jsautokw.h',               # Included in the body of frontend/TokenStream.h
>      'jswin.h',                  # Must be #included before <psapi.h>
> +    'machine/endian.h',         # breaks NPOTB platform if reorders includes (#932991)

I'd change the comment to this:

  # Must be included after <sys/types.h> on OpenBSD

::: js/src/config/check_spidermonkey_style.py
@@ +94,5 @@
>  oddly_ordered_inclnames = set([
>      'ctypes/typedefs.h',        # Included multiple times in the body of ctypes/CTypes.h
>      'jsautokw.h',               # Included in the body of frontend/TokenStream.h
>      'jswin.h',                  # Must be #included before <psapi.h>
> +    'machine/endian.h',         # breaks NPOTB platform if reorders includes (#932991)

Ditto.
Attachment #826705 - Flags: review?(n.nethercote) → review+
Seems mergetool didnt mark this as FIXED when it migrated to central..

https://hg.mozilla.org/mozilla-central/rev/7b9765059205
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.