Last Comment Bug 676857 - Make it a compile-time error to have a script-implementable interface with more methods than xptcall stubs can handle
: Make it a compile-time error to have a script-implementable interface with mo...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla8
Assigned To: Joshua Cranmer [:jcranmer]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-05 09:33 PDT by Boris Zbarsky [:bz]
Modified: 2011-08-14 05:18 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
First cut (24.51 KB, patch)
2011-08-05 11:08 PDT, Joshua Cranmer [:jcranmer]
no flags Details | Diff | Review
Patch, version 1 (25.62 KB, patch)
2011-08-05 11:42 PDT, Joshua Cranmer [:jcranmer]
benjamin: review+
Details | Diff | Review

Description Boris Zbarsky [:bz] 2011-08-05 09:33:18 PDT
xptcall is capped at something like 249 methods.  I ran into that today with some DOM interfaces and orange builds.... but that code should just have not compiled instead of dying on pure virtual calls, imo.
Comment 1 Joshua Cranmer [:jcranmer] 2011-08-05 11:01:34 PDT
I have a patch which:
1. Causes an error with xpidl
2. Prevents the interface from being loaded with xpt

(un)Fortunately, nsIDOMCSS2Properties has 469 methods on my machine, so it's a good check to make sure that we actually fail hard in these cases.
Comment 2 Joshua Cranmer [:jcranmer] 2011-08-05 11:08:54 PDT
Created attachment 551089 [details] [diff] [review]
First cut

This is a first cut at the bug. bz suggested over IRC that I elide the check in case of builtinclass interfaces. Since the real problem is over whether or not in can be stubbed, I'm also going to track down the stub generation code and make that fail.
Comment 3 Joshua Cranmer [:jcranmer] 2011-08-05 11:15:00 PDT
http://dxr.mozilla.org/mozilla/mozilla-central/xpcom/reflect/xptcall/src/xptcall.cpp.html#l69

Builtinclasses already fail the stub generation, so there shouldn't be a problem to failing to load that xpt file.
Comment 4 Joshua Cranmer [:jcranmer] 2011-08-05 11:42:13 PDT
Created attachment 551103 [details] [diff] [review]
Patch, version 1

This omits the error in case of builtinclass.
Comment 5 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-08-07 19:17:29 PDT
(In reply to Boris Zbarsky (:bz) from comment #0)
> xptcall is capped at something like 249 methods.  I ran into that today with
> some DOM interfaces and orange builds.... but that code should just have not
> compiled instead of dying on pure virtual calls, imo.

When you say 'xptcall' is capped, you mean the stubs, and not NS_InvokeByIndex, right?
Comment 6 Boris Zbarsky [:bz] 2011-08-07 19:21:23 PDT
I mean the stubs, yes.
Comment 7 Justin Wood (:Callek) 2011-08-09 03:48:34 PDT
Just a few days, and I can't import this to m-i atm, clearing checkin-needed until patch is updated.
Comment 8 Joshua Cranmer [:jcranmer] 2011-08-13 19:54:43 PDT
I refreshed the patch and pushed to mozilla-inbound...
Comment 9 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-08-14 05:18:39 PDT
http://hg.mozilla.org/mozilla-central/rev/16107d140423

Note You need to log in before you can comment on or make changes to this bug.