1476845 - Document how poison values are chosen and added

Status: RESOLVED FIXED

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

Description

[Paul Bone [:pbone]]

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.

Comment 1

[Paul Bone [:pbone]]

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.

Comment 2

[Paul Bone [:pbone]]

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.

Comment 3

[Paul Bone [:pbone]]

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.

Comment 4

[Steve Fink [:sfink] [:s:]]

(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).

Comment 5

[Paul Bone [:pbone]]

Attachment: Bug 1476845 - Update IsThingPoisoned r=jonco

Not all the listed posion values were used in IsThingPoisoned

Comment 6

[Paul Bone [:pbone]]

Attachment: Bug 1476845 - Collect poison values from around the engine into jsutil.h r=jonco

This is still WIP pending a needinfo from Waldo.

Depends on D33449

Comment 7

[Paul Bone [:pbone]]

Attachment: Bug 1476845 - Add link to mrgiggles' source r=jonco

Depends on D33450

Comment 8

[Paul Bone [:pbone]]

Attachment: Bug 1476845 - Define the LifoAlloc poison values in jsutil.h r=jonco

Depends on D33451

Comment 9

[Jeff Walden [:Waldo]]

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
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

Comment 11

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/78097b6cb93b
https://hg.mozilla.org/mozilla-central/rev/3d4f4d3f7fd5
https://hg.mozilla.org/mozilla-central/rev/acb4e093d0d2
https://hg.mozilla.org/mozilla-central/rev/23a7da77e315