Build failure when JS_TRACER is not defined

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: florian, Assigned: eric.hennigan)

Tracking

Trunk
All
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(3 attachments, 4 obsolete attachments)

Reporter

Description

9 years ago
Posted patch patchSplinter Review
build/mozilla/js/src/jscntxt.cpp: In member function 'bool JSThreadData::init()':
build/mozilla/js/src/jscntxt.cpp:505: error: 'maxCodeCacheBytes' was not declared in this scope

In jscntxt.h, maxCodeCacheBytes is declared in an #ifdef JS_TRACER, so I guess this build failure is just caused by a missing #ifdef in jscntxt.cpp.
Attachment #500761 - Flags: review?(gal)
Assignee

Comment 1

9 years ago
Assuming that Florian's patch is the correct approach there are two more stumbling blocks:

../jscompartment.cpp: In function ‘bool ScriptPoolDestroyed(JSContext*, js::mjit::JITScript*, uint32, uint32&)’:
../jscompartment.cpp:372: error: ‘ExecutablePool’ is not a member of ‘JSC’
../jscompartment.cpp:372: error: ‘pool’ was not declared in this scope
../jscompartment.cpp:372: error: invalid use of incomplete type ‘struct js::mjit::JITScript’
../jsinterp.h:106: error: forward declaration of ‘struct js::mjit::JITScript’

which can be solved with a similar patch2.diff

../jsdbgapi.cpp: In function ‘JSBool js_SetSingleStepMode(JSContext*, JSScript*, JSBool)’:
../jsdbgapi.cpp:189: error: ‘struct JSScript’ has no member named ‘singleStepMode’
../jsdbgapi.cpp: At global scope:
../jsdbgapi.cpp:120: warning: ‘void PurgeCallICs(JSContext*, JSScript*)’ defined but not used

solved with another patch3.diff
Assignee

Comment 2

9 years ago
Posted patch patch ifdef in jscompartment (obsolete) — Splinter Review
Attachment #500920 - Flags: review?
Assignee

Comment 3

9 years ago
Posted patch patch ifdef in jsdbgapi (obsolete) — Splinter Review

Comment 4

9 years ago
Comment on attachment 500761 [details] [diff] [review]
patch

JS_TRACER is enabled on all builds we ship, so disabling that line doesn't change any bits we ship => NPOTB. Feel free to land the patch.
Attachment #500761 - Flags: review?(gal) → review-

Updated

9 years ago
Attachment #500761 - Flags: review- → review+

Comment 5

9 years ago
Comment on attachment 500920 [details] [diff] [review]
patch ifdef in jscompartment

The ifdef looks unbalanced. Did you mean to put it after the }?

Comment 6

9 years ago
Comment on attachment 500921 [details] [diff] [review]
patch ifdef in jsdbgapi

Same here. NPOTB. Can land.
Attachment #500921 - Flags: review+
Assignee

Comment 7

9 years ago
(In reply to comment #5)
> Comment on attachment 500920 [details] [diff] [review]
> patch ifdef in jscompartment
> 
> The ifdef looks unbalanced. Did you mean to put it after the }?

I does look that way in the patch. The ifdef surrounds the entire ScriptPoolDestroyed function, including the preamble comment.
Reporter

Comment 8

9 years ago
(In reply to comment #1)
> Assuming that Florian's patch is the correct approach there are two more
> stumbling blocks:
> 
> ../jscompartment.cpp: In function ‘bool ScriptPoolDestroyed(JSContext*,
> js::mjit::JITScript*, uint32, uint32&)’:
> ../jscompartment.cpp:372: error: ‘ExecutablePool’ is not a member of ‘JSC’
> ../jscompartment.cpp:372: error: ‘pool’ was not declared in this scope
> ../jscompartment.cpp:372: error: invalid use of incomplete type ‘struct
> js::mjit::JITScript’
> ../jsinterp.h:106: error: forward declaration of ‘struct js::mjit::JITScript’
> 
> which can be solved with a similar patch2.diff
> 
> ../jsdbgapi.cpp: In function ‘JSBool js_SetSingleStepMode(JSContext*,
> JSScript*, JSBool)’:
> ../jsdbgapi.cpp:189: error: ‘struct JSScript’ has no member named
> ‘singleStepMode’
> ../jsdbgapi.cpp: At global scope:
> ../jsdbgapi.cpp:120: warning: ‘void PurgeCallICs(JSContext*, JSScript*)’
> defined but not used
> 
> solved with another patch3.diff

The patch I attached in comment 0 was enough to fix my build on PPC (yesterday at least...).
Which build configuration are you seeing this with?
Assignee

Comment 9

9 years ago
(In reply to comment #8)
> 
> The patch I attached in comment 0 was enough to fix my build on PPC (yesterday
> at least...).
> Which build configuration are you seeing this with?

I was seeing all three problems with changeset 59652:06351b16cca1 on a lixux 64bit system.

Updated

9 years ago
No longer blocks: 584860

Updated

9 years ago
Blocks: 584860
Assignee: general → eric.hennigan
Status: NEW → ASSIGNED
Hardware: PowerPC → All
Are attachments 500761 500921 ready to land Eric?(In reply to comment #7)
> (In reply to comment #5)
> > Comment on attachment 500920 [details] [diff] [review]
> > patch ifdef in jscompartment
> > 
> > The ifdef looks unbalanced. Did you mean to put it after the }?
> 
> I does look that way in the patch. The ifdef surrounds the entire
> ScriptPoolDestroyed function, including the preamble comment.

Please use "hg diff -p -U 8" in order to get more information in the changeset.

ScriptPoolDestroyed is only called with 
#if defined JS_METHODJIT && defined JS_MONOIC

Let's make it consistent.

The other 2 patches are ready to land?
Assignee

Comment 11

9 years ago
Posted file patch2.diff (obsolete) —
Attachment #500920 - Attachment is obsolete: true
Attachment #500920 - Flags: review?
Assignee

Comment 12

9 years ago
Posted file patch3.diff (obsolete) —
Attachment #500921 - Attachment is obsolete: true
Assignee

Comment 13

9 years ago
(In reply to comment #10)

> ScriptPoolDestroyed is only called with 
> #if defined JS_METHODJIT && defined JS_MONOIC
> 
> Let's make it consistent.
> 
> The other 2 patches are ready to land?

patch2.diff and patch3.diff have been extended with "-U8".
patch2.diff incorporates the JS_MONOIC change.

both are now ready to land.
Assignee

Comment 14

9 years ago
Posted patch patch2.diffSplinter Review
Forgot to check the 'patch' box.
Assignee

Comment 15

9 years ago
Posted patch patch3.diffSplinter Review
forgot to check the 'patch' box
Attachment #501376 - Attachment is obsolete: true
Attachment #501377 - Attachment is obsolete: true
Attachment #501403 - Flags: review+
Reporter

Updated

9 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/4dadcbc5c41e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.