530012 - [OS/2] build break in nsPresArena.cpp

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Rich Walsh]

Attachment: OS/2-specific code for nsPresArena.cpp

This file fails to compile due to a lack of OS/2-specific memory handling code.  The attached patch adds it.

Note:  tests\TestPoisonArea.cpp probably needs similar code to compile on OS/2.  I'll look into it when I can.

Comment 1

[Dave Yeo]

I:/comm-central/mozilla/layout/base/nsPresArena.cpp: In function 'bool ProbeRegion(PRUword, PRUword)':
I:/comm-central/mozilla/layout/base/nsPresArena.cpp:148: error: expected ')' before '{' token
I:/comm-central/mozilla/layout/base/nsPresArena.cpp:157: error: expected primary-expression before '}' token
I:/comm-central/mozilla/layout/base/nsPresArena.cpp:157: error: expected ';' before '}' token
I:/comm-central/mozilla/layout/base/nsPresArena.cpp:157: warning: no return statement in function returning non-void

Seems the patch needs

  unsigned long limit;
  if (DosQuerySysInfo(QSV_VIRTUALADDRESSLIMIT, QSV_VIRTUALADDRESSLIMIT,
-                      (void*)&limit, sizeof(limit)) {
+                      (void*)&limit, sizeof(limit))) {
    limit = 0x20000000;

Comment 2

[Rich Walsh]

Attachment: nsPresArena - v2 for 1.9.2

This adds OS/2 support to nsPresArena.cpp & tests/TestPoisonArea.cpp.

I selected 0xfffd0000 as the base address for the "poison memory" area because addresses from 0xfffc0000 to the end aren't mapped to anything (at least for the 4.5 kernel).  So, when you're looking at a register dump, if you see 0xfffd07ff in a general purpose register or 0xfffd0800 in EIP, you'll know that something tried accessing memory that had already been freed.

Comment 3

[Walter Meinl]

(In reply to comment #2)
> Created an attachment (id=416205) [details]
> nsPresArena - v2
Rich, your test passes ok, everything as expected? How about asking zweinberg for review?
[E:\mozbuild1\layout\base\tests]..\..\..\DIST\BIN\TESTPOISONAREA
INFO | negative control allocated at 0x02520000
INFO | positive control allocated at 0x02530000
INFO | poison area allocated at 0xfffd0000 (consolation prize)
TEST-PASS | reading negative control
TEST-PASS | executing negative control
TEST-PASS | writing negative control
TEST-PASS | reading positive control | exception code c0000005
TEST-PASS | executing positive control | exception code c0000005
TEST-PASS | writing positive control | exception code c0000005
TEST-PASS | reading poison area | exception code c0000005
TEST-PASS | executing poison area | exception code c0000005
TEST-PASS | writing poison area | exception code c0000005

Comment 4

[Walter Meinl]

Rich, ping, we should really get your patch reviewed, since we need it for FF-4.x as well as FF-3.6.x. Your patch builds fine and the log of the test is documented in comment #3.

Comment 5

[Rich Walsh]

Attachment: nsPresArena - v3

update to fix bitrot

Comment 6

[Rich Walsh]

(In reply to comment #3)
> everything as expected?

Given that all the tests pass and this patch has been in use by our nightlies for a year now without any problems, perhaps you could review and approve it.

Comment 7

[Walter Meinl]

Comment on attachment 500154 [details] [diff] [review]
nsPresArena - v3

The code added is all in ifdef XP_OS2 sections, should be zero risk for the tier1 platforms.

Comment 8

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/324bf35e0a9d

Comment 9

[Walter Meinl]

Comment on attachment 416205 [details] [diff] [review]
nsPresArena - v2 for 1.9.2

just revived v2 of the patch since it is needed for OS/2 on 1.9.2.x (the difference is just an output of the test).
The risk for tier_1 is zero as everything is correctly ifdefed XP_OS2 proven by the checkin of v3 for the trunk.

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 416205 [details] [diff] [review]
nsPresArena - v2 for 1.9.2

Approved for 1.9.2.15, a=dveditz for release-drivers

Comment 11

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/6ee566914e65

Comment 12

[Daniel Veditz [:dveditz]]

The "3.6.15" we're releasing today does not fix this bug, the release containing this bug fix has been renamed to "3.6.16" and the bugzilla flags will be updated to reflect that soon. Today's release is a re-release of 3.6.14 plus a fix for a bug that prevented many Java applets from starting up.