1359313 - stylo: whitelist moz-border-*-colors related bindings to satisfy heap write analysis

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Comment 1

[Jeremy Chen ([e-mail])]

Attachment: Bug 1359313 - whitelist -moz-border-*-colors related bindings to satisfy heap write analysis.

The -moz-border-*-colors bindings trigger errors because they're using outparams
which we don't handle yet. So, let's add them to the whitelist for now.

Review commit: https://reviewboard.mozilla.org/r/133330/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/133330/

Comment 2

[Cameron McCormack (:heycam)]

Comment on attachment 8861340 [details]
Bug 1359313 - whitelist -moz-border-*-colors related bindings to satisfy heap write analysis.

https://reviewboard.mozilla.org/r/133330/#review136122

Comment 3

[Jeremy Chen ([e-mail])]

A quick fix for hazard bustage by increase the NUM_ALLOWED_WRITE_HAZARDS is pushed in bug 1348173 comment 37. In this bug, we shall do the actual fix and restore the NUM_ALLOWED_WRITE_HAZARDS.

Comment 4

[Jeremy Chen ([e-mail])]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e5a0162d61613dcae5002b7e68add2cfcda2f02
Looks like adding outparams whitelist is not enought, since we further pass nsStyleBorder struct to set/clear its members. I'll try adding nsStyleBorder to the class/struct whitelist and see if this could be a fix.

Comment 5

[Jeremy Chen ([e-mail])]

Comment on attachment 8861340 [details]
Bug 1359313 - whitelist -moz-border-*-colors related bindings to satisfy heap write analysis.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/133330/diff/1-2/

Comment 6

[Bobby Holley (:bholley)]

We shouldn't be whitelisting entire classes, we should be whitelisting specific parameters as being writable.

Comment 7

[Jeremy Chen ([e-mail])]

Comment on attachment 8861340 [details]
Bug 1359313 - whitelist -moz-border-*-colors related bindings to satisfy heap write analysis.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/133330/diff/2-3/

Comment 8

[Jeremy Chen ([e-mail])]

try looks good with the latest patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d97fb7a067eb9a91ce90ba122fcf82b634193c5f

Per Bobby's comment (comment 6), I'm not sure if this is the right fix, so request another round of review from Bobby.

Comment 9

[Bobby Holley (:bholley)]

Comment on attachment 8861340 [details]
Bug 1359313 - whitelist -moz-border-*-colors related bindings to satisfy heap write analysis.

https://reviewboard.mozilla.org/r/133330/#review136976

I would expect these to live in treatAsSafeArgument, where we would just whitelist the arg for writing, rather than whitelisting the function completely (which is dangerous).

Comment 10

[Bobby Holley (:bholley)]

Comment on attachment 8861340 [details]
Bug 1359313 - whitelist -moz-border-*-colors related bindings to satisfy heap write analysis.

https://reviewboard.mozilla.org/r/133330/#review137140

Per IRC, move these up section so that the indirection explanation is clearer, and add some more detailed comments. r=me with that

Comment 11

[Jeremy Chen ([e-mail])]

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #10)
> Comment on attachment 8861340 [details]
> Bug 1359313 - whitelist -moz-border-*-colors related bindings to satisfy
> heap write analysis.
> 
> https://reviewboard.mozilla.org/r/133330/#review137140
> 
> Per IRC, move these up section so that the indirection explanation is
> clearer, and add some more detailed comments. r=me with that

Thank you for the review. :)

Since the patch is not modified (just moving couple lines up), so we shall reuse the try result in comment 4. I'm landing this.

Comment 12

[Jeremy Chen ([e-mail])]

Comment on attachment 8861340 [details]
Bug 1359313 - whitelist -moz-border-*-colors related bindings to satisfy heap write analysis.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/133330/diff/3-4/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7964174edf5f
whitelist -moz-border-*-colors related bindings to satisfy heap write analysis. r=bholley,heycam

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7964174edf5f