64bit safe code for mozilla/content (WinXP AMD64)

RESOLVED FIXED

Status

()

Core
DOM
--
enhancement
RESOLVED FIXED
15 years ago
14 years ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

Trunk
x86
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

15 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031208
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031208

Curretly code is defined as pointer type.

mozilla/content/base/public/nsIContent.h
...
  #endif

  protected:
    typedef long PtrBits;
    ^^^^^^^^^^^^^^^^^^^^^

     // Subclasses may use the low two bits of mParentPtrBits to store other data
     enum { kParentBitMask = 0x3 };
...

But WinXP 64bits for AMD64 is that "long" type is defined as 32 bits, so pointer
cast is always invalied.  So you should replace it with PRWord.  (PRWord is
pointer value).

Reproducible: Always

Steps to Reproduce:
not need.  This is porting issue
Actual Results:  
not need.  This is porting issue

Expected Results:  
not need.  This is porting issue
(Assignee)

Comment 1

15 years ago
Created attachment 138166 [details] [diff] [review]
mozilla/content diffs
PRWord is currently typedef-ed to |long|, so I don't see how this will help.
In any case, this is jst/peterv stuff, not misc layout code.
Assignee: nobody → general
Component: Layout: Misc Code → DOM
QA Contact: core.layout.misc-code → ian
Perhaps PRWord should be typedeffed to long long as needed (a configure check)?
(Assignee)

Comment 5

15 years ago
please see bug #226094 and bug #225859 about pointer structure of PR* and js*.

Comment 6

15 years ago
http://landfill.mozilla.org/mxr-test/nspr/source/nsprpub/pr/include/prtypes.h#472
462 ** WARNING: The undocumented data types PRWord and PRUword are
463 ** only used in the garbage collection and arena code.  Do not
464 ** use PRWord and PRUword in new code.
465 **
466 ** A PRWord is an integer that is the same size as a void*.
467 ** It implements the notion of a "word" in the Java Virtual
468 ** Machine.  (See Sec. 3.4 "Words", The Java Virtual Machine
469 ** Specification, Addison-Wesley, September 1996.
470 ** http://java.sun.com/docs/books/vmspec/index.html.)
471 */
472 typedef long PRWord;
473 typedef unsigned long PRUword;
First, this bug should not depend on bug 226094.  Rather, all of this bug, that
bug, and bug 225859 should depend on a meta-bug about supporting LLP64
compilation targets (of which WinXP on AMD64 is the first to come to our attention).

Second, let's fix this right, once.  We should use PRWord (PRUword if signed
comparison or sign extension would do the wrong thing) consistently in our code,
through whatever local typedef names are appropriate.  But this requires
changing NSPR, and changing deprecated NSPR types, to boot.  Cc'ing Wan-Teh.

/be
So js/src/jstypes.h is an old, ad-hoc fork of prtypes.h, done to make JS stand
alone whether built into Mozilla or not.  Therefore we'd need autoconf tests for
two places: NSPR and JS.  On the plus side, the JSWord (jsword, using the older
scalar-typedef lowercase naming convention) type and its unsigned counterpart
are not deprecated.

Wan-Teh, can we un-deprecate PRWord/PRUword and parameterize them for LLP64 as
well as LP64?  I hope you're reading bugmail -- if so, Happy New Year.

/be
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

15 years ago
Blocks: 237202
Comment on attachment 138166 [details] [diff] [review]
mozilla/content diffs

r+sr=jst
Attachment #138166 - Flags: superreview+
Attachment #138166 - Flags: review+
Note that there are other places that use the same typedef. Both the nsAttrValue
and nsAttrAndChildArray does.
(Assignee)

Updated

15 years ago
Assignee: general → m_kato
(Assignee)

Comment 11

15 years ago
checked in
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
Er.... we're in freeze.  Checkins require driver approval, no?
Yes, Makoto, this shouldn't have been checked in w/o approval from drivers. In
the future, pay attention to the rules. (for this particular change, I don't
think it's worth backing out or even worrying drivers with it since it really
doesn't change anything on the tier-1 platforms).

Comment 14

15 years ago
Re: comment 7 and comment 8

As long as we precisely define PRWord and PRUword,
that should be fine.  I think what you want is the
new intptr_t and uintptr_t types in C99.

What I don't like about PRWord and PRUword are:

1. "Word" has many meanings.  To the best of my
knowledge, the "word" in PRWord means a word in
the Java virtual machine (the predecessor of NSPR
was used by Netscape to port Sun's JVM to other
platforms), and most people don't know what exactly
a JVM word means.

2. PRWord is defined to be an integer that is the
same size as a void*.  "The same size" might be
overly restrictive.  It may be better to say "large
enough to hold".
> 2. PRWord is defined to be an integer that is the
> same size as a void*.  "The same size" might be
> overly restrictive.  It may be better to say "large
> enough to hold".

Actually, in many cases we do want something that is "The same size". "large
enough to hold" might mean that it's bigger then a void* which might cause
unneccesary bloat.

Does intptr_t and uintptr_t work across enough compilers for us to use in XP
mozilla code? If not we *need* something that is defined to be "The same size"
as a void* or we'll keep running into this problem.
Note that it's still not fixed for the nsAttrValue and nsAttrName classes.
(Assignee)

Comment 16

15 years ago
About discuss PRWord value, please see bug 226094 and bug 225859.  Since PRWord
is defined as pointer, we should use it, not long.

And about other code of Mozilla/content, I file a new bug as bug 239499.

Comment 17

15 years ago
Jonas: intptr_t and uintptr_t are relatively new and
therefore are not universally supported yet.  Also,
they are "large enough to hold", as opposed to "the
same size as", a pointer.
Then it sounds like intptr_t and uintptr_t won't cut it here and we need some
NSPR typedef that we can rely on.
You need to log in before you can comment on or make changes to this bug.