Closed Bug 380998 Opened 14 years ago Closed 14 years ago

StackGrowthDirection is not reliable with Sun Studio 11

Categories

(Core :: JavaScript Engine, defect)

x86
OpenSolaris
defect
Not set
major

Tracking

()

RESOLVED FIXED

People

(Reporter: ginnchen+exoracle, Assigned: crowderbt)

References

Details

(Keywords: fixed1.8.0.13, fixed1.8.1.5)

Attachments

(1 file, 2 obsolete files)

StackGrowthDirection in jscpucfg.c is not reliable with Sun Studio 11 on Solaris x86.

Sun Studio doesn't support "__attribute__((noinline))"
Compile it with optimization will get JS_STACK_GROWTH_DIRECTION (1)
should be JS_STACK_GROWTH_DIRECTION (-1)

It may cause problems with some javascripts.
some options we may use,

1) don't call StackGrowthDirection(), hardcode JS_STACK_GROWTH_DIRECTION for Solaris
2) use "-g" to compile jscpucfg.c, it will avoid inline for Sun Studio.
3) put StackGrowthDirection() into another C file, maybe we can avoid it be inlined for all/most compilers
4) use a function pointer for StackGrowthDirection(), maybe we can avoid it be inlined for all/most compilers
Attachment #265231 - Flags: review?(brendan)
at least we should use MOZ_DEBUG_FLAGS instead of "-g"
Attachment #265231 - Attachment is obsolete: true
Attachment #265231 - Flags: review?(brendan)
Attachment #265237 - Flags: review?(brendan)
It would be better to define NS_NEVER_INLINE to something that your compiler respects -- is there such a pragma?

$ grep NS_NEVER_INLINE *.[ch]
jscpucfg.c:#define NS_NEVER_INLINE __attribute__((noinline))
jscpucfg.c:#define NS_NEVER_INLINE
jscpucfg.c:static int NS_NEVER_INLINE StackGrowthDirection(int *dummy1addr)

/be
I forgot Sun Studio C does have no_inline pragma.
But it's not as same as gcc.
I've to declare the function first.

Sun Studio 12 will have same attributes ability as gcc.
Attachment #265477 - Flags: review?
Attachment #265477 - Flags: review? → review?(brendan)
Attachment #265237 - Attachment is obsolete: true
Attachment #265237 - Flags: review?(brendan)
Comment on attachment 265477 [details] [diff] [review]
patch that uses #pragma for Sun Studio

r=me, thanks.

/be
Attachment #265477 - Flags: review?(brendan) → review+
Comment on attachment 265477 [details] [diff] [review]
patch that uses #pragma for Sun Studio

brendan, can you commit this for me?

Also, I think it's important for 1.5.x and 2.0.x.

BTW:
#pragma no_inline only works for Sun Studio C compiler not C++.
"-g" option only disables inlining for Sun Studio C++ compiler.
So this approach is the correct one.
Attachment #265477 - Flags: approval1.8.1.5?
Attachment #265477 - Flags: approval1.8.0.13?
jscpucfg.c: 3.26
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Flags: blocking1.8.1.5?
Flags: blocking1.8.0.13?
This should certainly not block a branch release, in my opinion.
Not blocking _us_, but Sun (and other vendors) are committed to longer branch support than Mozilla.
Flags: blocking1.8.1.5?
Flags: blocking1.8.1.5+
Flags: blocking1.8.0.13?
Flags: blocking1.8.0.13+
Comment on attachment 265477 [details] [diff] [review]
patch that uses #pragma for Sun Studio

Approved for 1.8.1.5 and 1.8.0.13, a=dveditz for release-drivers
Attachment #265477 - Flags: approval1.8.1.5?
Attachment #265477 - Flags: approval1.8.1.5+
Attachment #265477 - Flags: approval1.8.0.13?
Attachment #265477 - Flags: approval1.8.0.13+
Whiteboard: [checkin to 1_8 and 1_8_0 branch needed]
Whiteboard: [checkin to 1_8 and 1_8_0 branch needed] → [checkin needed (1_8 and 1_8_0)]
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Probably this should be assigned to crowder, who did the checkin, rather than to ginn.chen.

/be
Assignee: general → crowder
Status: REOPENED → NEW
Status: NEW → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
MOZILLA_1_8_BRANCH: jscpucfg.c: 3.25.2.1
MOZILLA_1_8_0_BRANCH: jscpucfg.c: 3.25.10.1
Whiteboard: [checkin needed (1_8 and 1_8_0)]
Duplicate of this bug: 377909
You need to log in before you can comment on or make changes to this bug.