Add support for the QNX operating system.

Add support for the QNX operating system.

This patch contains contributions from the following members of the
BlackBerry Web Technologies team:

Eli Fidler <efidler@blackberry.com>;
Konrad Piascik <kpiascik@blackberry.com>;
Jeff Rogers <jrogers@blackberry.com>;
Cosmin Truta <ctruta@blackberry.com>;
Peter Wang <peter.wang@torchmobile.com.cn>;
Xiaobo Wang <xiaobwang@blackberry.com>;
Ming Xie <mxie@blackberry.com>;
Leo Yang <leoyang@blackberry.com>;

R=bmeurer@chromium.org, jkummerow@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=18430

[c.truta, 2013-11-13 08:34:01]

Dear V8 reviewers,

I submitted, on behalf of my team at BlackBerry, a patch that contains the
initial support for the QNX operating system, and I would like to kindly ask for
a review.

We are developing our future web engine based on Chromium, and this patch is one
of our first steps. It makes the QNX port functional, and we have more changes
(fixes and improvements) planned to submit upstream, pending your initial
feedback.

We prepared this submission based on the documentation found on the V8 home
page, as well as the Chromium home page, and we will be highly appreciative of
any other insight that you could give to us.

Sincerely,
Cosmin Truta

[Benedikt Meurer, 2013-11-15 11:49:58]

Hey Cosmin,

Thanks for the patch. Here's the first round of comments.

One important question: Do you plan on porting Chromium to QNX?

greets,
Benedikt

https://codereview.chromium.org/61153009/diff/1/include/v8config.h
File include/v8config.h (right):

https://codereview.chromium.org/61153009/diff/1/include/v8config.h#newcode91
include/v8config.h:91: //  V8_OS_SOLARIS       - Sun Solaris and OpenSolaris
Add V8_OS_QNX here.

https://codereview.chromium.org/61153009/diff/1/src/atomicops_internals_arm_g...
File src/atomicops_internals_arm_gcc.h (right):

https://codereview.chromium.org/61153009/diff/1/src/atomicops_internals_arm_g...
src/atomicops_internals_arm_gcc.h:35: #if V8_OS_QNX
The atomicops_*.h files were copied from Chromium's base/atomicops_*.h. If
possible we'd like to avoid adding our own changes here. Do you plan on porting
Chromium to QNX as well?

https://codereview.chromium.org/61153009/diff/1/src/checks.cc
File src/checks.cc (right):

https://codereview.chromium.org/61153009/diff/1/src/checks.cc#newcode35
src/checks.cc:35: #endif  // V8_LIBC_GLIBC || V8_OS_BSD || V8_OS_QNX
The #endif should only list the conditions from #if.

https://codereview.chromium.org/61153009/diff/1/src/checks.cc#newcode89
src/checks.cc:89: #endif  // V8_LIBC_GLIBC || V8_OS_BSD || V8_OS_QNX
Same as above.

https://codereview.chromium.org/61153009/diff/1/src/cpu.cc
File src/cpu.cc (right):

https://codereview.chromium.org/61153009/diff/1/src/cpu.cc#newcode86
src/cpu.cc:86: #if V8_HOST_ARCH_ARM
Better change this #if to #if V8_HOST_ARCH_ARM && V8_OS_LINUX, as it is really
linux specific.

https://codereview.chromium.org/61153009/diff/1/src/cpu.cc#newcode364
src/cpu.cc:364: #else  // !V8_OS_QNX
Use #elif V8_OS_LINUX here.

https://codereview.chromium.org/61153009/diff/1/src/platform-posix.cc
File src/platform-posix.cc (right):

https://codereview.chromium.org/61153009/diff/1/src/platform-posix.cc#newcode257
src/platform-posix.cc:257: #if defined(V8_OS_QNX)
Use #if V8_OS_QNX.

https://codereview.chromium.org/61153009/diff/1/src/platform-qnx.cc
File src/platform-qnx.cc (right):

https://codereview.chromium.org/61153009/diff/1/src/platform-qnx.cc#newcode134
src/platform-qnx.cc:134: const size_t msize = RoundUp(requested,
sysconf(_SC_PAGESIZE));
Why not AllocateAlignment() here?

https://codereview.chromium.org/61153009/diff/1/src/platform-qnx.cc#newcode264
src/platform-qnx.cc:264: void OS::SignalCodeMovingGC() {
Is there any support for perf/ll_prof in QNX? Otherwise you should just leave
this empty.

https://codereview.chromium.org/61153009/diff/1/src/sampler.cc
File src/sampler.cc (right):

https://codereview.chromium.org/61153009/diff/1/src/sampler.cc#newcode432
src/sampler.cc:432: #error Host architecture x64 is not supported on QNX.
We don't add #error's here.

[Jakob Kummerow, 2013-11-15 12:08:15]

Unrelated to the code: I don't see any mention of BlackBerry on the list of
people/companies who have signed the CLA. That's a prerequisite for us to be
able to accept your contributions. See
https://code.google.com/p/v8/wiki/Contributing#The_small_print.

[efidler1, 2013-11-15 15:45:58]

We have, it's under RIM.

[Jakob Kummerow, 2013-11-15 16:05:42]

Under RIM we have Eli Fidler and Jeff Rogers as authorized contributors. AFAIK
our lawyers really like it when every contributor is listed.

[c.truta, 2013-11-18 13:36:32]

Pending the resolution of the remaining CLA-related issues, I am submitting the
2nd version of the changelist for review.

I held off the change to atomicops_internals_arm_gcc.h for now. If that can land
via Chromium's base/atomicops_* instead, it's fine for us. And the answer to
Benedikt's question is yes :-)

https://codereview.chromium.org/61153009/diff/1/src/atomicops_internals_arm_g...
File src/atomicops_internals_arm_gcc.h (right):

https://codereview.chromium.org/61153009/diff/1/src/atomicops_internals_arm_g...
src/atomicops_internals_arm_gcc.h:35: #if V8_OS_QNX
> The atomicops_*.h files were copied from Chromium's base/atomicops_*.h. If
possible we'd like to avoid adding our own changes here. Do you plan on porting
Chromium to QNX as well?

Yes. We can keep this change for now in our private branches, until we land it
in Chromium, and then resubmit here, if you prefer.

https://codereview.chromium.org/61153009/diff/1/src/cpu.cc
File src/cpu.cc (right):

https://codereview.chromium.org/61153009/diff/1/src/cpu.cc#newcode86
src/cpu.cc:86: #if V8_HOST_ARCH_ARM
I couldn't change it all the way to #if V8_HOST_ARCH_ARM && V8_OS_LINUX, because
LINUX && MIPS further below would have been left out; but I did replace
!V8_OS_QNX with V8_OS_LINUX.

https://codereview.chromium.org/61153009/diff/1/src/platform-posix.cc
File src/platform-posix.cc (right):

https://codereview.chromium.org/61153009/diff/1/src/platform-posix.cc#newcode257
src/platform-posix.cc:257: #if defined(V8_OS_QNX)
> Use #if V8_OS_QNX.
D'oh! Oversight. Thank you.
I also added #if V8_OS_LINUX || ... because I noticed that, in Linux, it is
recommended to use sysconf instead of getpagesize, just as in QNX.

https://codereview.chromium.org/61153009/diff/1/src/platform-qnx.cc
File src/platform-qnx.cc (right):

https://codereview.chromium.org/61153009/diff/1/src/platform-qnx.cc#newcode134
src/platform-qnx.cc:134: const size_t msize = RoundUp(requested,
sysconf(_SC_PAGESIZE));
> Why not AllocateAlignment() here?
Wasn't deliberate. Fixed.

[Benedikt Meurer, 2013-11-19 07:01:27]

https://codereview.chromium.org/61153009/diff/1/src/atomicops_internals_arm_g...
File src/atomicops_internals_arm_gcc.h (right):

https://codereview.chromium.org/61153009/diff/1/src/atomicops_internals_arm_g...
src/atomicops_internals_arm_gcc.h:35: #if V8_OS_QNX
On 2013/11/18 13:36:32, cosmin.truta wrote:
> > The atomicops_*.h files were copied from Chromium's base/atomicops_*.h. If
> possible we'd like to avoid adding our own changes here. Do you plan on
porting
> Chromium to QNX as well?
> 
> Yes. We can keep this change for now in our private branches, until we land it
> in Chromium, and then resubmit here, if you prefer.

That's great! I think we'll just sync all atomicops_* files again, once your
changes have landed.

https://codereview.chromium.org/61153009/diff/1/src/platform-posix.cc
File src/platform-posix.cc (right):

https://codereview.chromium.org/61153009/diff/1/src/platform-posix.cc#newcode257
src/platform-posix.cc:257: #if defined(V8_OS_QNX)
On 2013/11/18 13:36:32, cosmin.truta wrote:
> > Use #if V8_OS_QNX.
> D'oh! Oversight. Thank you.
> I also added #if V8_OS_LINUX || ... because I noticed that, in Linux, it is
> recommended to use sysconf instead of getpagesize, just as in QNX.

sysconf(_SC_PAGESIZE) just calls getpagesize() for glibc, ulibc and bionic, so
there's no reason add special case for Linux here. What is the reason to use
sysconf() on QNX?

[c.truta, 2013-11-20 14:18:29]

On 2013/11/19 07:01:27, Benedikt Meurer wrote:
> sysconf(_SC_PAGESIZE) just calls getpagesize() for glibc, ulibc and bionic, so
> there's no reason add special case for Linux here. What is the reason to use
> sysconf() on QNX?

getpagesize() returns int, and that might not represent large values correctly.
As stated in the QNX docs, such as, for example, here:
http://www.qnx.com/developers/docs/6.5.0/index.jsp?topic=%2Fcom.qnx.doc.neutr...

I don't foresee a platform that would actually have >2GB page size in any near
future, but I thought I should follow the book regardless. The Linux man page on
getpagesize() also recommends using sysconf instead, for different reasons. But
for any practical purpose, I am fine without that particular change, and I can
remove it if you wish.

[c.truta, 2013-11-20 14:28:15]

On 2013/11/20 14:18:29, cosmin.truta wrote:
> getpagesize() returns int, and that might not represent large values
correctly.
> As stated in the QNX docs, such as, for example, here:
>
http://www.qnx.com/developers/docs/6.5.0/index.jsp?topic=%252Fcom.qnx.doc.neu...

The link I pasted previously doesn't work because Rietveld mangled the URL
percent encoding... Retrying.
http://www.qnx.com/developers/docs/6.5.0/index.jsp?topic=/com.qnx.doc.neutrin...

[Benedikt Meurer, 2013-11-21 06:08:25]

On 2013/11/20 14:28:15, cosmin.truta wrote:
> On 2013/11/20 14:18:29, cosmin.truta wrote:
> > getpagesize() returns int, and that might not represent large values
> correctly.
> > As stated in the QNX docs, such as, for example, here:
> >
>
http://www.qnx.com/developers/docs/6.5.0/index.jsp?topic=%25252Fcom.qnx.doc.n...
> 
> The link I pasted previously doesn't work because Rietveld mangled the URL
> percent encoding... Retrying.
>
http://www.qnx.com/developers/docs/6.5.0/index.jsp?topic=/com.qnx.doc.neutrin...

Ok, then we should probably just use sysconf(_SC_PAGESIZE) on all POSIX
platforms. Please update this CL to replace getpagesize() with
sysconf(_SC_PAGESIZE).

[c.truta, 2013-12-12 06:26:07]

Hi, everyone,

I uploaded the last incarnation of the QNX port, addressing all past review
comments, and including some minor GYP updates.

To my understanding, the remaining CLA issues have been resolved. Could you
please review this patch, and if it's okay, could you please also land it for
me?

[Benedikt Meurer, 2013-12-12 07:11:04]

LGTM.

Jakob, please take a look at the gyp changes.

[Jakob Kummerow, 2013-12-12 13:21:29]

GYP changes LGTM.

[c.truta, 2013-12-18 04:13:54]

Thank you very much for your reviews. I see the build tree is back to green, and
I set the commit flag. Is there any further action required on my part, in order
to have this landed?

[Sven Panne, 2013-12-18 07:15:37]

On 2013/12/18 04:13:54, cosmin.truta wrote:
> Thank you very much for your reviews. I see the build tree is back to green,
and
> I set the commit flag. Is there any further action required on my part, in
order
> to have this landed?

Nope, just patience. :-) We have dozens of CLs which wait to be landed, but
unless we want total chaos we have to land them incrementally over the next few
days/weeks...

[Jakob Kummerow, 2013-12-18 10:34:47]

On 2013/12/18 07:15:37, Sven Panne wrote:
> On 2013/12/18 04:13:54, cosmin.truta wrote:
> > Thank you very much for your reviews. I see the build tree is back to green,
> and
> > I set the commit flag. Is there any further action required on my part, in
> order
> > to have this landed?
> 
> Nope, just patience. :-) We have dozens of CLs which wait to be landed, but
> unless we want total chaos we have to land them incrementally over the next
few
> days/weeks...

Also, the commit checkbox doesn't do anything (useful) (yet) for V8 CLs.

[c.truta, 2013-12-19 06:39:30]

Sven Panne wrote:
> > Nope, just patience. :-) We have dozens of CLs which wait to be landed, but
> > unless we want total chaos we have to land them incrementally over the next
> > few days/weeks...

Jakob wrote:
> Also, the commit checkbox doesn't do anything (useful) (yet) for V8 CLs.

Thank you for all the info, this is useful for me to learn how things get done
here.

[Benedikt Meurer, 2014-01-02 07:04:19]

Committed patchset #3 manually as r18430 (presubmit successful).