crash in macro CCMAP_HAS_CHAR_EXT

VERIFIED FIXED in mozilla0.9.9

Status

()

Core
Internationalization
--
critical
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: Shanjian Li, Assigned: Shanjian Li)

Tracking

({crash, intl})

Trunk
mozilla0.9.9
x86
Windows NT
crash, intl
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

16 years ago
Accessing to above page, global font will be resorted. If Code2001 Font is installed, 
there will be a crash.
(Assignee)

Comment 1

16 years ago
Created attachment 65268 [details] [diff] [review]
patch

The problem is in the way this macro is written. If the space before and after
operator "&" is 
omitted, unexpected thing might happen.
(Assignee)

Comment 2

16 years ago
add frank to cc list. Frank, could you r=?
Status: NEW → ASSIGNED

Comment 3

16 years ago
do not understand your patch. Why that will make any difference. 
Should you also change
323              #define CCMAP_SURROGATE_FLAG         0X0001 

to
323              #define CCMAP_SURROGATE_FLAG         0x0001 

the "X" to "x"

Comment 4

16 years ago
Please explain to me what will happen before your change and what will happen
after your chang in details so I can understand your patch. 

according to page 65 (section 2.4.1 Integer Constants) of "The C++ Programming
Langaue Second Edition" by Bjarne Strousstrup. it said

A constant startings with zero followed by x (0x) is a hexadecimal (base 16)
number,....

It didn't mention X. But in page 8 (2.5.1 Integer Constants) of "The Annotated
C++ Reference Manul" by Margaret A. Ellis & Bjarne Stroustrup, it said:
A sequence of digits preceded by 0x or 0X is taken to be a hexadecimal integer
(baes sixteen)....

and it DOES mention X.

So maybe x or X have no impact on this.

Comment 5

16 years ago
What really make the difference?

changing "a&0X0001 && b" to "a & 0X0001 && b" or 
changing it to "(a&0X001) && b" ?
(Assignee)

Comment 6

16 years ago
Created attachment 65284 [details] [diff] [review]
repost patch
Attachment #65268 - Attachment is obsolete: true
(Assignee)

Comment 7

16 years ago
frank, 
I have difficulty understand it too. The difference is made by following part:
"(CCMAP_FLAG(m)&CCMAP_SURROGATE_FLAG)". Mozilla have run time memory access 
voilation in mentioned website. After I change it to "(CCMAP_FLAG(m) & CCMAP_SURROGATE_FLAG)",
everything works fine again. So I guessed the problem was because '&' could not 
interpreted in several meanings, (reference, bitand, address). My original 
macro without space might confused compiler. 0x0001 and 0X0001 does not make 
difference in my test. 
(Assignee)

Comment 8

16 years ago
frank, rbs, roy, 
Could any of you give a code review?
(Assignee)

Comment 9

16 years ago
Sorry, ignore my previous post. I mean to post to another bug.

Updated

16 years ago
Severity: normal → critical
Keywords: crash

Updated

16 years ago
Keywords: intl
QA Contact: ruixu → ylong
(Assignee)

Updated

16 years ago
Target Milestone: --- → mozilla0.9.9
(Assignee)

Comment 10

16 years ago
*** Bug 123881 has been marked as a duplicate of this bug. ***

Comment 11

16 years ago
yea I run into this too in http://bugzilla.mozilla.org/show_bug.cgi?id=123881
looks like same thing.
I still cannot explain the difference between with space and without but I 
definitely see the difference here.
(Assignee)

Comment 12

16 years ago
Why old macro crash:
  For character in BMP, "!((ucs4)&0xffff0000)" will be true. If
"CCMAP_HAS_CHAR(m, (PRUnichar)(ucs4))" is false, the macro will check next part, ie,
"CCMAP_FLAG(m)&CCMAP_SURROGATE_FLAG && CCMAP_HAS_CHAR(CCMAP_FOR_PLANE_EXT((m),
CCMAP_PLANE(ucs4)), (ucs4)&0xffff)". 

The later part is supposed to only use for checking extension map. Plane "0"
will lead to check extension plane "-1", which does not exist. 

Comment 13

16 years ago
Comment on attachment 65284 [details] [diff] [review]
repost patch

r=ftang
Attachment #65284 - Flags: review+
(Assignee)

Comment 14

16 years ago
Created attachment 68172 [details] [diff] [review]
add ext macro test as suggested by ftang
Attachment #65284 - Attachment is obsolete: true
(Assignee)

Comment 15

16 years ago
Comment on attachment 68172 [details] [diff] [review]
add ext macro test as suggested by ftang

assume r=ftang since this is suggested by him.
Attachment #68172 - Flags: review+
(Assignee)

Comment 16

16 years ago
Marc, could you sr ?

Comment 17

16 years ago
Comment on attachment 68172 [details] [diff] [review]
add ext macro test as suggested by ftang

sr=attinasi
Attachment #68172 - Flags: superreview+
(Assignee)

Comment 18

16 years ago
fix checked in. 
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 19

16 years ago
The original URL http://www.lupa.cz/anketa.phtml is not existing any more.

I checked the URL that in bug 123881, and I don't see crash there.

So I'm marking it as verified here,please re-open if some one still see the crash.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.