1161932 - Silent a coverity warning in nsBidi.cpp

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

coverity thinks that there is an important issue in bug 1157726 (CID 1296737)

On this line:
https://dxr.mozilla.org/mozilla-central/source/layout/base/nsBidi.cpp?from=layout/base/nsBidi.cpp#782
coverity says:
   CID 1296737 (#1 of 1): Out-of-bounds read (OVERRUN)158. overrun-local: Overrunning array stack of 127 2-byte elements at element index 4294967295 (byte offset 8589934590) using index stackLast (which evaluates to 4294967295).

Comment 1

[Ted Clancy [:tedders1]]

Attachment: bug-1161932-coverity-warning.patch

Hopefully this will stop the warnings.

Comment 2

[Simon Montagu :smontagu]

Comment on attachment 8604397 [details] [diff] [review]
bug-1161932-coverity-warning.patch

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

::: layout/base/nsBidi.cpp
@@ +773,5 @@
>                /* pop embedding entries        */
>                /* until the last isolate entry */
>                stackLast--;
> +
> +              // Since validIsolateCount is true, there must be an isolate entry 

Nit: trailing whitespace

Comment 3

[Ted Clancy [:tedders1]]

Green treeherder: https://treeherder.mozilla.org/#/jobs?repo=try&revision=328bc608cf19

Comment 4

[Ted Clancy [:tedders1]]

Attachment: bug-1161932-coverity-warning.patch

Fixed nit.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/10eeb41f2674

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/10eeb41f2674

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/10eeb41f2674

Comment 8

[Ted Clancy [:tedders1]]

This is needed for Bug 1166203, which is a confirmed blocker for 2.2.

Comment 9

[Ted Clancy [:tedders1]]

Comment on attachment 8605014 [details] [diff] [review]
bug-1161932-coverity-warning.patch

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
RTL support for B2G (Bug 906270)

This patch is required for Bug 1166203, which is confirmed as blocking 2.2. (And probably a bunch of similar issues which haven't been spotted yet.)

User impact if declined: 
Punctuation marks will appear in the wrong place when LTR phrases appear within RTL text, or vice versa.

Testing completed: 
Green treeherder run - https://treeherder.mozilla.org/#/jobs?repo=try&revision=328bc608cf19

Risk to taking this patch (and alternatives if risky): 
None forseen.

String or UUID changes made by this patch:
None.

Comment 10

[Josh Cheng [:josh]]

Hi Ryan,
This is the 3rd bug. Next one is 1163583.
Can you give me access to bug 1163583 for approval?
Thanks!

Comment 11

[Ryan VanderMeulen [:RyanVM]]

(In reply to Josh Cheng [:josh] from comment #10)
> Hi Ryan,
> This is the 3rd bug. Next one is 1163583.
> Can you give me access to bug 1163583 for approval?
> Thanks!

Wrong email address there. Regardless, it has auto-approval anyway :). Also, FWIW, standard practice is to uplift patches in the same order that they landed on m-c.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/bf4e182b1e75