MIPS: Fix `This CL enables precise source positions for all V8 compilers`

MIPS: Fix `This CL enables precise source positions for all V8 compilers`

Fix c3a6ca68d0646b10885ef7017557eaf463db2e4a

Fix compilation failure on MIPS and GCC cross compile that started to appear
after the CL c3a6ca68d0646b10885ef7017557eaf463db2e4a landed. The compilation
error is due to:

.././src/objects-inl.h:4129:54: error: assuming signed overflow does not occur
when assuming that (X + c) < X is always false [-Werror=strict-overflow]
   DCHECK(index >= 0 && length >= 0 && index + length >= index &&

BUG=

Committed: https://crrev.com/ff4513a69bde7d913df04aa82e2dbfa562e70684
Cr-Commit-Position: refs/heads/master@{#41067}

[ivica.bogosavljevic, 2016-11-15 10:59:04]

Description was changed from

==========
MIPS: Fix `This CL enables precise source positions for all V8 compilers`

Fix c3a6ca68d0646b10885ef7017557eaf463db2e4a

Fix compilation failure on MIPS and GCC crosscompile that started to appear
after the CL c3a6ca68d0646b10885ef7017557eaf463db2e4a landed.

BUG=
==========

to

==========
MIPS: Fix `This CL enables precise source positions for all V8 compilers`

Fix c3a6ca68d0646b10885ef7017557eaf463db2e4a

Fix compilation failure on MIPS and GCC cross compile that started to appear
after the CL c3a6ca68d0646b10885ef7017557eaf463db2e4a landed. The compilation
error is due to:

.././src/objects-inl.h:4129:54: error: assuming signed overflow does not occur
when assuming that (X + c) < X is always false [-Werror=strict-overflow]
   DCHECK(index >= 0 && length >= 0 && index + length >= index &&


BUG=
==========

[ivica.bogosavljevic, 2016-11-15 11:02:26]

ivica.bogosavljevic@imgtec.com changed reviewers:
+ michael.starzinger@chromium.org, tebbi@chromium.org, vogelheim@chromium.org,
yangguo@chromium.org

[ivica.bogosavljevic, 2016-11-15 11:02:27]

One line fix for the compilation failure on MIPS.

Please take a look!

[vogelheim, 2016-11-15 17:27:19]

On 2016/11/15 11:02:27, ivica.bogosavljevic wrote:
> One line fix for the compilation failure on MIPS.
> 
> Please take a look!

I'm not sure it's legitimate to eliminate the overflow check. I hope one of the
other reviewers (Yang? Michael?) would have a more qualified opinion.

[ivica.bogosavljevic, 2016-11-16 08:49:17]

On 2016/11/15 17:27:19, vogelheim wrote:
> On 2016/11/15 11:02:27, ivica.bogosavljevic wrote:
> > One line fix for the compilation failure on MIPS.
> > 
> > Please take a look!
> 
> I'm not sure it's legitimate to eliminate the overflow check. I hope one of
the
> other reviewers (Yang? Michael?) would have a more qualified opinion.

According to C standard, overflow on signed integers causes an undefined
behavior
(I think this is legacy from the time when signed values were not represented
as 2s complement).

There is a way to check if addition of two signed integer would overflow
without actually performing the addition:

overflow happens if:
index + length > MAX_INT
<=>
index > MAX_INT - length

since length is positive, MAX_INT - length never underflows so this is the
only check we need

[ivica.bogosavljevic, 2016-11-16 14:17:24]

Reminder!

Please notice that because of this none of the tests for MIPS arch in the Debug
configuration are not executing.

[Michael Starzinger, 2016-11-16 14:34:20]

mstarzinger@chromium.org changed reviewers:
+ clemensh@chromium.org, mstarzinger@chromium.org
- michael.starzinger@chromium.org

[Michael Starzinger, 2016-11-16 14:34:20]

Looking OK from my end. I would like to get Clemens' blessing since he was the
original author of the assertion in question.

+Clemens: PTAL.

[Clemens Hammacher, 2016-11-16 14:40:42]

On 2016/11/16 at 14:34:20, mstarzinger wrote:
> Looking OK from my end. I would like to get Clemens' blessing since he was the
original author of the assertion in question.
> 
> +Clemens: PTAL.

This was indeed meant as overflow check, so can we just replace it by "length <=
kMaxInt - offset"?

[Clemens Hammacher, 2016-11-16 14:41:18]

On 2016/11/16 at 14:40:42, Clemens Hammacher wrote:
> On 2016/11/16 at 14:34:20, mstarzinger wrote:
> > Looking OK from my end. I would like to get Clemens' blessing since he was
the original author of the assertion in question.
> > 
> > +Clemens: PTAL.
> 
> This was indeed meant as overflow check, so can we just replace it by "length
<= kMaxInt - offset"?

Sorry, replace offset by index.

[ivica.bogosavljevic, 2016-11-16 16:09:12]

PTAL

[Clemens Hammacher, 2016-11-16 16:46:00]

The CQ bit was checked by clemensh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-16 16:46:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Clemens Hammacher, 2016-11-16 16:46:13]

lgtm

[commit-bot: I haz the power, 2016-11-16 17:13:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-16 17:13:45]

Dry run: This issue passed the CQ dry run.

[ivica.bogosavljevic, 2016-11-17 09:52:01]

The CQ bit was checked by ivica.bogosavljevic@imgtec.com

[commit-bot: I haz the power, 2016-11-17 09:52:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-17 09:55:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-17 09:55:38]

Try jobs failed on following builders:
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/28759)

[ivica.bogosavljevic, 2016-11-17 10:04:45]

I tried to land it, but I am missing LGTM from the owner of object-inl.h

[titzer, 2016-11-17 10:07:09]

titzer@chromium.org changed reviewers:
+ titzer@chromium.org

[titzer, 2016-11-17 10:07:09]

lgtm

[Clemens Hammacher, 2016-11-17 10:07:56]

The CQ bit was checked by clemensh@chromium.org

[commit-bot: I haz the power, 2016-11-17 10:08:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-17 10:10:26]

Description was changed from

==========
MIPS: Fix `This CL enables precise source positions for all V8 compilers`

Fix c3a6ca68d0646b10885ef7017557eaf463db2e4a

Fix compilation failure on MIPS and GCC cross compile that started to appear
after the CL c3a6ca68d0646b10885ef7017557eaf463db2e4a landed. The compilation
error is due to:

.././src/objects-inl.h:4129:54: error: assuming signed overflow does not occur
when assuming that (X + c) < X is always false [-Werror=strict-overflow]
   DCHECK(index >= 0 && length >= 0 && index + length >= index &&


BUG=
==========

to

==========
MIPS: Fix `This CL enables precise source positions for all V8 compilers`

Fix c3a6ca68d0646b10885ef7017557eaf463db2e4a

Fix compilation failure on MIPS and GCC cross compile that started to appear
after the CL c3a6ca68d0646b10885ef7017557eaf463db2e4a landed. The compilation
error is due to:

.././src/objects-inl.h:4129:54: error: assuming signed overflow does not occur
when assuming that (X + c) < X is always false [-Werror=strict-overflow]
   DCHECK(index >= 0 && length >= 0 && index + length >= index &&


BUG=
==========

[commit-bot: I haz the power, 2016-11-17 10:10:27]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-17 22:37:52]

Description was changed from

==========
MIPS: Fix `This CL enables precise source positions for all V8 compilers`

Fix c3a6ca68d0646b10885ef7017557eaf463db2e4a

Fix compilation failure on MIPS and GCC cross compile that started to appear
after the CL c3a6ca68d0646b10885ef7017557eaf463db2e4a landed. The compilation
error is due to:

.././src/objects-inl.h:4129:54: error: assuming signed overflow does not occur
when assuming that (X + c) < X is always false [-Werror=strict-overflow]
   DCHECK(index >= 0 && length >= 0 && index + length >= index &&


BUG=
==========

to

==========
MIPS: Fix `This CL enables precise source positions for all V8 compilers`

Fix c3a6ca68d0646b10885ef7017557eaf463db2e4a

Fix compilation failure on MIPS and GCC cross compile that started to appear
after the CL c3a6ca68d0646b10885ef7017557eaf463db2e4a landed. The compilation
error is due to:

.././src/objects-inl.h:4129:54: error: assuming signed overflow does not occur
when assuming that (X + c) < X is always false [-Werror=strict-overflow]
   DCHECK(index >= 0 && length >= 0 && index + length >= index &&

BUG=

Committed: https://crrev.com/ff4513a69bde7d913df04aa82e2dbfa562e70684
Cr-Commit-Position: refs/heads/master@{#41067}
==========

[commit-bot: I haz the power, 2016-11-17 22:37:53]

Patchset 2 (id:??) landed as
https://crrev.com/ff4513a69bde7d913df04aa82e2dbfa562e70684
Cr-Commit-Position: refs/heads/master@{#41067}