92134 - Sun Workshop 6 Update 2 _FCS_ fails to build due "jscpucfg" error

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Roland Mainz]

I just tested the new Sun Workshop 6 Update 2 _FCS_ (I was using Sun Workshop 6
Update 2 EarlyAccess2 before that).

The build fails with:
-- snip --
/opt/SUNWspro/bin/cc -I/usr/local/include -DOSTYPE=\"SunOS5\" -DOSARCH=\"SunOS\"
-DMOZ_REFLOW_PERF -DMOZ_REFLOW_PERF_DSP -DOJI -DEXPORT_JS_API 
-DJS_USE_SAFE_ARENA
-I/home/mozilla/builds/2001-07-23-08-trunk/objdir_ws6_xlib/dist/include/nspr  -o
jscpucfg ../../../../../src/2001-07-23-08-trunk/mozilla/js/src/jscpucfg.c
./jscpucfg > jsautocfg.tmp
./jscpucfg: unknown byte order!
-- snip --

The fix is easy - Sun Workshop simply sticks some vars into free registers is
possible. I'll file a patch to get rid of that...

Comment 1

[Roland Mainz]

target milestone 0.9.3.
I know that this is this ||-close - but a lot of people will start to move to
Update 2 _FCS_ (either from Update 1 or Update 2 EA2) - and that will be
doomsday for those people.

Patch follows.

Comment 2

[Roland Mainz]

Attachment: Patch for 2001-07-23-08-trunk...

Comment 3

[Roland Mainz]

Filed patch.

The patch simple makes the |union|s |volatile| and |static| to avoid that the
compiler does any optimisations with it.

The use of |union| in jscpucfg.c is _ILLEGAL_ per ANSI/ISO 9899-1990. Sun
Workshop 6 Update 2 is correct in the case that it can cache members of an
union.

But it is (AFAIK) a bug that it still sticks the var in a register when
|volatile| is used (therefore I used |static| here to force it).

Comment 4

[Brendan Eich [:brendan]]

Looks ok to me, apart from the 80th column being violated by that repeated
comment!  Roland, please testify that this patch works on other tier-1 and -2
Mozilla platforms.  If so, r/sr=[e-mail].

/be

Comment 5

[Roland Mainz]

mkaply:
Wanna test this patch with OS/2 and add comment here if it works/fails, please ?

Comment 6

[Mike Kaply [:mkaply]]

OS/2: Patch applies and compiles and provides the same output for both 
the original jscpucfg.c and the new one.

Comment 7

[Donnie Cranford]

I also can attest that this patch works to fix the problem for Forte 6 UD2 
FCS. I have tried it on Solaris 8 04/01 on  a Ultra 10

Comment 8

[cls]

Works on linux & beos.  jscpucfg isn't used on mac or win32.  If OS/2 passes,
then aix & hpux should pass as well. r=cls

Comment 9

[Roland Mainz]

Requesting a= for trunk and branch.

Comment 10

[Roland Mainz]

Attachment: New patch 2001-07-27-08-trunk with fixed comments

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

a=dbaron (on behalf of drivers) for trunk checkin during 0.9.3 closure

Comment 12

[Roland Mainz]

CC:'ing mkaply for checkin, please...

Comment 13

[Brendan Eich [:brendan]]

bryner: why is volatile needed here, but not in the jsnum.h macro you added
recently to disambiguate memory for gcc 3.0?

/be

Comment 14

[Roland Mainz]

brendan:
Normal variables which are non-|volatile|/|static| are of "storage type" |auto|
- which can be in memory and/or register. Therefore Sun Workshop is correct when
it puts some members of an |union| in a register. Members of an |union| can only
be used exclusive-or, e.g. writing in member 'x' does not neccesarily change
member 'y' (assuming both are of storage type |auto|, e.g. the default storage
type).
Unfortunately Sun Workshop does not honor that |volatile| for some reason
(bug!?), therefore I used |static| to force that beast to do it.

A better solution would be to use pointers of different datatypes to the same
piece of memory instead of using |union|s the "hack-way"...

Comment 15

[Brendan Eich [:brendan]]

gisburn: I know all about volatile, static, auto, etc.  The question I asked
bryner, not you, was: why do we not need volatile in a similar case in jsnum.h,
for gcc 3.0?

BTW, your claim that union is hacky and that differently-typed pointers
constitute a better solution falls over because ISO C does not require compilers
to consider that pointers to different types might alias the same memory
location(s), and therefore, compilers are free to reorder instructions using
these pointers.

/be

Comment 16

[Roland Mainz]

Patch checked-in by timeless
(http://bonsai.mozilla.org/cvsquery.cgi?module=MozillaTinderboxAll&branch=HEAD&cvsroot=/cvsroot&date=explicit&mindate=996474780&maxdate=996475500&who=timeless%25mac.com).

Marking bug as FIXED.

Comment 17

[Phil Schwartau]

Marking Verified - 

Comment 18

[Brian Ryner (not reading)]

brendan: I don't know the answer to that, but I'll bet drepper does.

Comment 19

[Ulrich Drepper]

ISO C99 introduces aliasing but without giving clear guidance as to how to
resolve the problem.  The definition of the restrict semantics completely rules
out using pointers of different types to the same object.  This is what was
corrected in jsnum recently.  I know that Sun relies on this but they are wrong.

gcc shows to not bend this rule (since it would disable valuable optimizations)
and instead define the union semantics as already described.  ISO C says

  6.7.2.1 [...]
  The value of at most one of the members can be stored in a union object at any
time.

The standard also explains the layout of a union and how storing effects the
bytes belonging to other union members.  This leaves almost no surprises.  This
is why gcc allows (and recommends) access to different union members without
allowing the compiler to deduce aliasing information.

It's simply the Sun's compiler and gcc don't have the same extension to solve a
problem which has no solution inside the ISO C standard.  It would be wise to
#ifdef the code based on the compiler used.  Alternative turn off the
optimizations which require these work-arounds.  gcc has -fno-strict-aliasing. 
I hope Sun was clever enough to introduce a similar flag.