StackGrowthDirection is not reliable with Sun Studio 11

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
major
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Ginn Chen, Assigned: Brian Crowder)

Tracking

({fixed1.8.0.13, fixed1.8.1.5})

Trunk
x86
OpenSolaris
fixed1.8.0.13, fixed1.8.1.5
Points:
---
Bug Flags:
blocking1.8.1.5 +
blocking1.8.0.13 +
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

10 years ago
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.
(Reporter)

Comment 1

10 years ago
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

Comment 2

10 years ago
Created attachment 265231 [details] [diff] [review]
Add "-g" to prevent  StackGrowthDirection from being inlined

Updated

10 years ago
Attachment #265231 - Flags: review?(brendan)
(Reporter)

Comment 3

10 years ago
at least we should use MOZ_DEBUG_FLAGS instead of "-g"

Updated

10 years ago
Attachment #265231 - Attachment is obsolete: true
Attachment #265231 - Flags: review?(brendan)

Comment 4

10 years ago
Created attachment 265237 [details] [diff] [review]
Use more general "MOZ_DEBUG_FLAGS" instead of "-g"
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
(Reporter)

Comment 6

10 years ago
Created attachment 265477 [details] [diff] [review]
patch that uses #pragma for Sun Studio

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?
(Reporter)

Updated

10 years ago
Attachment #265477 - Flags: review? → review?(brendan)

Updated

10 years ago
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+
(Reporter)

Comment 8

10 years ago
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?
(Assignee)

Comment 9

10 years ago
jscpucfg.c: 3.26
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Updated

10 years ago
Flags: in-testsuite-
(Reporter)

Updated

10 years ago
Flags: blocking1.8.1.5?
Flags: blocking1.8.0.13?
(Assignee)

Comment 10

10 years ago
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+
(Reporter)

Updated

10 years ago
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)]

Updated

10 years ago
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

Updated

10 years ago
Status: NEW → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED
(Assignee)

Comment 14

10 years ago
MOZILLA_1_8_BRANCH: jscpucfg.c: 3.25.2.1
MOZILLA_1_8_0_BRANCH: jscpucfg.c: 3.25.10.1
Keywords: fixed1.8.0.13, fixed1.8.1.5
Whiteboard: [checkin needed (1_8 and 1_8_0)]

Updated

10 years ago
Duplicate of this bug: 377909
You need to log in before you can comment on or make changes to this bug.