Closed Bug 1161932 Opened 4 years ago Closed 4 years ago

Silent a coverity warning in nsBidi.cpp

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox39 --- wontfix
firefox40 --- wontfix
firefox41 --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: Sylvestre, Assigned: tedders1)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: [CID 1296737])

Attachments

(1 file, 1 obsolete file)

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).
Assignee: nobody → tclancy
Summary: Silent a warning warning → Silent a coverity warning
Summary: Silent a coverity warning → Silent a coverity warning in nsBidi.cpp
Hopefully this will stop the warnings.
Attachment #8604397 - Flags: review?(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
Attachment #8604397 - Flags: review?(smontagu) → review+
Fixed nit.
Attachment #8604397 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/10eeb41f2674
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
This is needed for Bug 1166203, which is a confirmed blocker for 2.2.
blocking-b2g: --- → 2.2?
Blocks: 1166203
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.
Attachment #8605014 - Flags: approval-mozilla-b2g37?
Hi Ryan,
This is the 3rd bug. Next one is 1163583.
Can you give me access to bug 1163583 for approval?
Thanks!
Flags: needinfo?(ryanvanderzanden)
Attachment #8605014 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
(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.
Flags: needinfo?(ryanvanderzanden)
blocking-b2g: 2.2? → ---
You need to log in before you can comment on or make changes to this bug.