1206105 - nth-child integer overflow with crash in [@ nthChildGenericMatches ]

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[SkyLined]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150826023504

Steps to reproduce:

Load the following html in Firefox
<style>*:nth-child(-n-2147483647){



Actual results:

Integer overflow exception


Expected results:

Nothing.

Comment 1

[Loic]

Attachment: 1206105.html (instant crash)

Comment 2

[Loic]

CR: https://crash-stats.mozilla.com/report/index/a48e8639-f0d4-479e-b19a-c0f3f2150919

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Is it new that this is a crash rather than regular old "undefined behavior".

Comment 4

[Loic]

FF8 crashes so it is more an old undefined case.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Odd. I get a crash on Mac too:

  Program received signal EXC_ARITHMETIC, Arithmetic exception.

The debugger claims we're on this line:

  const int32_t n = (index - b) / a;

with index == 1, b == -2147483647 (so -INT32_MAX) and a == -1.  All three variables are int32_t.

The subtraction index - b is in fact undefined behavior here, since it's basically 1+INT32_MAX.  But what compilers seem to do with it in practice is just let it overflow in the obvious way, so we get INT32_MIN.

It's the division of INT32_MIN by -1 that raises EXC_ARITHMETIC.  That's probably undefined behavior too, though I haven't checked....

Comment 6

[Boris Zbarsky [:bzbarsky]]

Anyway, seems like we should just use a CheckedInt32 here, and "not match" if it overflows?  And check index-b for being INT32_MIN before dividing by -1.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: Use CheckedInt for an+b selector matching

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8664087 [details] [diff] [review]
Use CheckedInt for an+b selector matching

r=me

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9986f05fe61d

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/000fe1df3e59

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/000fe1df3e59