787933 - Stop using stdin types in IPC code

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Actual stdint types are nicer.

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Script

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part 1: Scripted changes

Generated by attachment 657850 [details].

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part 2: Fixes

Comment 4

[Landry Breuil (:gaston)]

openbsd builds are broken with these two patches, i'll work on a third one on top of them.

Comment 5

[Landry Breuil (:gaston)]

/home/landry/src/mozilla-central/ipc/chromium/src/base/singleton.h:118:77: error: no matching function for call to 'NoBarrier_Load(base::subtle::AtomicWord*&)'
/home/landry/src/mozilla-central/ipc/chromium/src/base/singleton.h:118:77: note: candidates are:
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:132:17: note: base::subtle::Atomic32 base::subtle::NoBarrier_Load(const volatile Atomic32*)
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:132:17: note:   no known conversion for argument 1 from 'base::subtle::AtomicWord* {aka long int*}' to 'const volatile Atomic32* {aka const volatile int*}'
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:236:17: note: base::subtle::Atomic64 base::subtle::NoBarrier_Load(const volatile Atomic64*) <near match>
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:236:17: note:   no known conversion for argument 1 from 'base::subtle::AtomicWord* {aka long int*}' to 'const volatile Atomic64* {aka const volatile long long int*}'
/home/landry/src/mozilla-central/ipc/chromium/src/base/singleton.h:125:65: error: no matching function for call to 'Acquire_CompareAndSwap(base::subtle::AtomicWord*&, int, const AtomicWord&)'
/home/landry/src/mozilla-central/ipc/chromium/src/base/singleton.h:125:65: note: candidates are:
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:71:17: note: base::subtle::Atomic32 base::subtle::Acquire_CompareAndSwap(volatile Atomic32*, base::subtle::Atomic32, base::subtle::Atomic32)
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:71:17: note:   no known conversion for argument 1 from 'base::subtle::AtomicWord* {aka long int*}' to 'volatile Atomic32* {aka volatile int*}'
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:199:17: note: base::subtle::Atomic64 base::subtle::Acquire_CompareAndSwap(volatile Atomic64*, base::subtle::Atomic64, base::subtle::Atomic64) <near match>
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:199:17: note:   no known conversion for argument 1 from 'base::subtle::AtomicWord* {aka long int*}' to 'volatile Atomic64* {aka volatile long long int*}'
/home/landry/src/mozilla-central/ipc/chromium/src/base/singleton.h:131:73: error: no matching function for call to 'Release_Store(base::subtle::AtomicWord*&, base::subtle::AtomicWord&)'
/home/landry/src/mozilla-central/ipc/chromium/src/base/singleton.h:131:73: note: candidates are:
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:126:13: note: void base::subtle::Release_Store(volatile Atomic32*, base::subtle::Atomic32)
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:126:13: note:   no known conversion for argument 1 from 'base::subtle::AtomicWord* {aka long int*}' to 'volatile Atomic32* {aka volatile int*}'
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:215:13: note: void base::subtle::Release_Store(volatile Atomic64*, base::subtle::Atomic64) <near match>
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:215:13: note:   no known conversion for argument 1 from 'base::subtle::AtomicWord* {aka long int*}' to 'volatile Atomic64* {aka volatile long long int*}'


First failure is a case of atomic type mismatching. Failure is on 64 bits, note the :

 48 #if defined(OS_OPENBSD) && !defined(ARCH_CPU_64_BITS)
 49 typedef Atomic32 AtomicWord;
 50 #else
 51 typedef intptr_t AtomicWord;
 52 #endif

snippet in atomicops.h.

so on OpenBSD/amd64, Atomic64 is int64_t (ie long long), and atomicWord is... intptr_t, ie long ? This time, it seems nspr types are not to blame..

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

How about if you change it to

#ifdef OS_OPENBSD
#ifdef ARCH_CPU_64_BITS
typedef Atomic64 AtomicWord;
#else
typedef Atomic32 AtomicWord;
#endif // ARCH_CPU_64_BITS
#else
typedef intptr_t AtomicWord;
#endif // OS_OPENBSD

?

Comment 7

[Landry Breuil (:gaston)]

Whoa, good catch. With that on top of your patchset, my amd64 builds are okay. Gotta check on my i386/32bit builder, but it's down atm, so don't hold the commit on that.

 // Use AtomicWord for a machine-sized pointer.  It will use the Atomic32 or
 // Atomic64 routines below, depending on your architecture.
-#if defined(OS_OPENBSD) && !defined(ARCH_CPU_64_BITS)
+#ifdef OS_OPENBSD
+#ifdef ARCH_CPU_64_BITS
+typedef Atomic64 AtomicWord;
+#else
 typedef Atomic32 AtomicWord;
+#endif // ARCH_CPU_64_BITS
 #else
 typedef intptr_t AtomicWord;
-#endif
+#endif // OS_OPENBSD

Comment 8

[Benjamin Smedberg]

Comment on attachment 657851 [details] [diff] [review]
Part 1: Scripted changes

cjones, this won't hork us any more than we already are with the chromium import, right?

Comment 9

[Landry Breuil (:gaston)]

Note that patch 1 fails to apply on inbound tip.

applying bug-787933-1
patching file dom/ipc/ContentParent.h
Hunk #1 FAILED at 107
1 out of 1 hunks FAILED -- saving rejects to file dom/ipc/ContentParent.h.rej
patching file ipc/glue/RPCChannel.cpp
Hunk #2 FAILED at 663
1 out of 2 hunks FAILED -- saving rejects to file ipc/glue/RPCChannel.cpp.rej

Comment 10

[Landry Breuil (:gaston)]

Attachment: typedef atomicword depending on 32/64bits on OpenBSD

Fixes the build on top of part 1&2 on OpenBSD.. might be debatable.

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 657851 [details] [diff] [review]
Part 1: Scripted changes

Nope, not to my knowledge.

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Landed all three patches in one changeset, because they didn't build on their own:

https://hg.mozilla.org/mozilla-central/rev/513941951140