Closed Bug 1566057 Opened 2 years ago Closed 2 years ago

Use BytecodeLocation and BytecodeIterator in `Detecting` function

Categories

(Core :: JavaScript Engine, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox71 --- fixed

People

(Reporter: mgaudet, Assigned: whawkins, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: good-first-bug, Whiteboard: [lang=c++])

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #1538557 +++

Background

Historically access to bytecode in SpiderMonkey has been fairly freeform and common. This means that common idioms are repeated without encapsulation, and it's difficult to audit the codebase for patterns which makes changes around bytecode much more fragile than we'd like.

We'd like to encapsulate manipulation of bytecode within the engine to a set of accessor classes where possible.

This Bug

By using the BytecodeLocation and BytecodeIterator, we'd like to replace uses of jsbytecode* and pcOffset inside Detecting.

This bug is only for changing the implementation of this function: It's OK that when values escape this function the return to jsbytecode* and pcOffsets. We can't change everything at once!

Note: It is expected that you may have to expand the interface of BytecodeLocation or BytecodeIterator. The interfaces as they exist now are driven by clients, and are comparatively thin.

Prior Art

  • In Bug 1499544, Part 2 used these interfaces to modify JSScript::assertValidJumpTargets. You can use that patch as inspiration for what we're looking for.

Prerequisites

Before getting started, you'll want to

  • You Have a checkout of the Firefox source code
  • Make sure you can build SpiderMonkey
  • Read this walkthrough about how development works in Firefox

Getting Help

Feel free to leave comments on this bug for questions, or, if you have more synchronous questions about this bug, feel free to drop into #jsapi on irc.mozilla.org.

Tips:

  • Not sure if the code you've been editing is getting run? Insert a call to MOZ_CRASH, a macro which will crash when executed, and run the entire test suite with an optimized build (for speed). If you see crashes, you can then use a debug build to make sure it's crashing in your code!

I have started to work in this bug.

Assignee: nobody → whawkins
Keywords: checkin-needed

Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/717694871f97
Convert Detecting function to use bytecode iterator. r=tcampbell

Keywords: checkin-needed
Backout by shindli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/499b56b9d670
Backed out changeset 717694871f97 for causing spidermonkey bustages CLOSED TREE

Updated patch to resolve this bustage is out for review.

Flags: needinfo?(whawkins)
Keywords: checkin-needed

Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3afa57585d5c
Convert Detecting function to use bytecode iterator. r=tcampbell

Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.