Closed Bug 1476845 Opened 6 years ago Closed 5 years ago

Document how poison values are chosen and added

Categories

(Core :: JavaScript: GC, enhancement, P3)

Product:
Core
Component:
JavaScript: GC
Version:
61 Branch
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla69
Tracking Flags:
Tracking Status
firefox69 --- fixed

People

(Reporter: pbone, Assigned: pbone)

References

(Blocks 2 open bugs)

Details

Attachments

(4 files)

Bug 1476845 - Update IsThingPoisoned r=jonco
(deleted), text/x-phabricator-request
Details
Bug 1476845 - Collect poison values from around the engine into jsutil.h r=jonco
(deleted), text/x-phabricator-request
Details
Bug 1476845 - Add link to mrgiggles' source r=jonco
(deleted), text/x-phabricator-request
Details
Bug 1476845 - Define the LifoAlloc poison values in jsutil.h r=jonco
(deleted), text/x-phabricator-request
Details
Document how poison values are choosen, where they are listed, and other rules. I don't know all these rules and assume that we list all the poison values in one (or two) places: https://searchfox.org/mozilla-central/rev/6f86cc3479f80ace97f62634e2c82a483d1ede40/js/src/jsutil.h#254-269 https://searchfox.org/mozilla-central/rev/6f86cc3479f80ace97f62634e2c82a483d1ede40/js/src/gc/Marking.cpp#131-143 See Bug 1475678 Comment 19 and Bug 1475678 Comment 22. We should also search for existing calls to JS_POISON and ensure they all confirm and all the poison values are known.
Priority: -- → P3

From discussion with the other GC folks and we remembered several places to find existing poison values.

Some poison values are hard coded values, others in jsutil,h, others in jemalloc, and at least one from emilio that mrgiggles knows about.

We should check for sentinal values also.

Hi Jeff,

https://searchfox.org/mozilla-central/diff/7e808a56db4a43e44cfa6e0d783c18934bf5aada/js/src/vm/Scope.h#168

You introduced the poisoning in the above code. I'm not familiar with this part of the interpreter, can you suggest a good name for the constant 0xCC here. I'm moving them all into jsutil.h.

Thanks.

Flags: needinfo?(jwalden)

Hi Steve,

I can't find the mrgiggles source code, I want to check that it's up-to-date his/its with all these poison values.

Thanks.

Flags: needinfo?(sphink)

(In reply to Paul Bone [:pbone] from comment #3)

Hi Steve,

I can't find the mrgiggles source code, I want to check that it's up-to-date his/its with all these poison values.

It's in the first line of the help output, https://bitbucket.org/sfink/mrgiggles but specifically see https://bitbucket.org/sfink/mrgiggles/src/34a279f9dfdeda0d095839a348c2ed1b73b75128/plugins/knowledge/__init__.py#lines-321:368 (and yes, it's all ugly and hardcoded).

Flags: needinfo?(sphink)
Summary: Document how poison values are choosen and added → Document how poison values are chosen and added

Not all the listed posion values were used in IsThingPoisoned

This is still WIP pending a needinfo from Waldo.

Depends on D33449

Depends on D33450

Depends on D33451

Assignee: nobody → pbone
Status: NEW → ASSIGNED

(In reply to Paul Bone [:pbone] from comment #2)

https://searchfox.org/mozilla-central/diff/7e808a56db4a43e44cfa6e0d783c18934bf5aada/js/src/vm/Scope.h#168

You introduced the poisoning in the above code. I'm not familiar with this part of the interpreter, can you suggest a good name for the constant 0xCC here. I'm moving them all into jsutil.h.

SCOPE_DATA_TRAILING_NAMES_POISON? Maybe?

Flags: needinfo?(jwalden)
Blocks: 1559059
Attachment #9069283 - Attachment description: Bug 1476845 - Collect poison values from around the engine into jsutil.h → Bug 1476845 - Collect poison values from around the engine into jsutil.h r=jonco
Blocks: 1559065
Pushed by pbone@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/78097b6cb93b Update IsThingPoisoned r=jonco https://hg.mozilla.org/integration/autoland/rev/3d4f4d3f7fd5 Collect poison values from around the engine into jsutil.h r=jonco https://hg.mozilla.org/integration/autoland/rev/acb4e093d0d2 Add link to mrgiggles' source r=jonco https://hg.mozilla.org/integration/autoland/rev/23a7da77e315 Define the LifoAlloc poison values in jsutil.h r=jonco
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: