VTable padding off by one

RESOLVED WONTFIX

Status

Tamarin
Virtual Machine
P3
enhancement
RESOLVED WONTFIX
7 years ago
6 years ago

People

(Reporter: pnkfelix, Assigned: pnkfelix)

Tracking

unspecified
Q1 12 - Brannan
Bug Flags:
flashplayer-injection -
flashplayer-qrb +
flashplayer-bug -

Details

VTable.h had a bool 'pad[3]' element to offset the bool 'linked' member (that padding was added in changeset:622 by Steven as part of a slew of fixes to structure alignment).

In changeset:3568 it was changed to a 'pad[2]' element to accommodate a new bool member, 'basecase', that Lars added to VTable.

Then in changeset:5923 the 'basecase' member was removed, but the padding was not re-adjusted back to 'pad[3]'

If there was ever a reason for the padding in the first place, then presumably the amount of padding should be adjusted back to 'pad[3]'

REFERENCES
[changeset:622]  http://hg.mozilla.org/tamarin-redux/diff/622/core/VTable.h
[changeset:3568] http://hg.mozilla.org/tamarin-redux/diff/3568/core/VTable.h
[changeset:5923] http://hg.mozilla.org/tamarin-redux/diff/5923/core/VTable.h

Comment 1

7 years ago
How did you discover this?  Inspection?  Is this important enough to add an assertion or self-test for this condition?

Updated

7 years ago
Assignee: nobody → stejohns
Flags: flashplayer-qrb+
Flags: flashplayer-injection-
Flags: flashplayer-bug-
Priority: -- → P3
Target Milestone: --- → Q3 11 - Serrano
(In reply to comment #1)
> How did you discover this?  Inspection?  Is this important enough to add an
> assertion or self-test for this condition?

tltRipley asked about why we had the 'pad[2]' member in the #tamarin chat room.

I was curious about it and couldn't explain its presence via inspection of the tip, so I used 'hg blame' [*] to find out where it was injected, and traced back the history to the individual change sets where it was introduced.

The real question is how Steven found the places to add padding to back in changeset:622.  (I suspect he did so via inspection, which would be unfortunate; but maybe I am wrong and there is a tool that can help us here.)

[*] well, really emacs 'M-x annotate', but same difference

Updated

7 years ago
Target Milestone: Q3 11 - Serrano → Q1 12 - Brannan

Comment 3

7 years ago
Totally by inspection. The explicit padding is arguably unnecessary.

Comment 4

7 years ago
From Felix:

It doesn't need to be resolved before Serrano.  Its absolutely deferrable; it is at most a performance issue, not a correctness one, and I suspect the performance difference is insignificant.
(Assignee)

Updated

6 years ago
Assignee: stejohns → fklockii

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.