(In reply to Rong Bao [:csmantle] from comment #10) > Thanks for the explanation. > > Another observation is that while `deadline` ([^1]), `poolEnd` ([^2]) are both counted in bytes, `secondaryVeneers` seems to be counted in `Inst`. I think we are also missing a `* InstSize` at ([^3]), right? Perhaps we should also mark this bug as regressed by Bug 1210554? > > [^1]: https://searchfox.org/firefox-main/rev/50445a0ee5bbeaeba68911b649963669e4c483e2/js/src/jit/shared/IonAssemblerBufferWithConstantPools.h#721 > [^2]: https://searchfox.org/firefox-main/rev/50445a0ee5bbeaeba68911b649963669e4c483e2/js/src/jit/shared/IonAssemblerBufferWithConstantPools.h#722-723 > [^3]: https://searchfox.org/firefox-main/rev/50445a0ee5bbeaeba68911b649963669e4c483e2/js/src/jit/shared/IonAssemblerBufferWithConstantPools.h#749-750 This is indeed correct! Good Catch! And for what is worth, this could potentially be leveraged on ARM and ARM64 as well. This issue is being addressed by the [Part 2 patch](https://phabricator.services.mozilla.com/D270942) attached on this bug, and we should consider back-porting it to all branches on a release train of Firefox. Multiple branch sizes could be registered, such that one of the veneer (trampoline between out-of-reached regions) can be corrupted to encode an address which is incorrect, leading to a jump at an unexpected location on ARM and ARM64.
Bug 1996840 Comment 16 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
(In reply to Rong Bao [:csmantle] from comment #10) > Thanks for the explanation. > > Another observation is that while `deadline` ([^1]), `poolEnd` ([^2]) are both counted in bytes, `secondaryVeneers` seems to be counted in `Inst`. I think we are also missing a `* InstSize` at ([^3]), right? Perhaps we should also mark this bug as regressed by Bug 1210554? > > [^1]: https://searchfox.org/firefox-main/rev/50445a0ee5bbeaeba68911b649963669e4c483e2/js/src/jit/shared/IonAssemblerBufferWithConstantPools.h#721 > [^2]: https://searchfox.org/firefox-main/rev/50445a0ee5bbeaeba68911b649963669e4c483e2/js/src/jit/shared/IonAssemblerBufferWithConstantPools.h#722-723 > [^3]: https://searchfox.org/firefox-main/rev/50445a0ee5bbeaeba68911b649963669e4c483e2/js/src/jit/shared/IonAssemblerBufferWithConstantPools.h#749-750 This is indeed correct! Good Catch! And for what is worth, this could potentially be leveraged on ARM64 as well. This issue is being addressed by the [Part 2 patch](https://phabricator.services.mozilla.com/D270942) attached on this bug, and we should consider back-porting it to all branches on a release train of Firefox. Multiple branch sizes could be registered, such that one of the veneer (trampoline between out-of-reached regions) can be corrupted to encode an address which is incorrect, leading to a jump at an unexpected location on ARM and ARM64. [edit: remove the fact that this can be leveraged on ARM, given that ARM no longer seems to be using veneer]
(In reply to Rong Bao [:csmantle] from comment #10) > Thanks for the explanation. > > Another observation is that while `deadline` ([^1]), `poolEnd` ([^2]) are both counted in bytes, `secondaryVeneers` seems to be counted in `Inst`. I think we are also missing a `* InstSize` at ([^3]), right? Perhaps we should also mark this bug as regressed by Bug 1210554? > > [^1]: https://searchfox.org/firefox-main/rev/50445a0ee5bbeaeba68911b649963669e4c483e2/js/src/jit/shared/IonAssemblerBufferWithConstantPools.h#721 > [^2]: https://searchfox.org/firefox-main/rev/50445a0ee5bbeaeba68911b649963669e4c483e2/js/src/jit/shared/IonAssemblerBufferWithConstantPools.h#722-723 > [^3]: https://searchfox.org/firefox-main/rev/50445a0ee5bbeaeba68911b649963669e4c483e2/js/src/jit/shared/IonAssemblerBufferWithConstantPools.h#749-750 This is indeed correct! Good Catch! And for what is worth, this could potentially be leveraged on ARM64 as well. This issue is being addressed by the [Part 2 patch](https://phabricator.services.mozilla.com/D270942) attached on this bug, and we should consider back-porting it to all branches on a release train of Firefox. Multiple branch sizes could be registered, such that one of the veneer (trampoline between out-of-reached regions) can be corrupted to encode an address which is incorrect, leading to a jump at an unexpected location on ARM64. [edit: remove the fact that this can be leveraged on ARM, given that ARM no longer seems to be using veneer]
(In reply to Rong Bao [:csmantle] from comment #10) > Thanks for the explanation. > > Another observation is that while `deadline` ([^1]), `poolEnd` ([^2]) are both counted in bytes, `secondaryVeneers` seems to be counted in `Inst`. I think we are also missing a `* InstSize` at ([^3]), right? Perhaps we should also mark this bug as regressed by Bug 1210554? > > [^1]: https://searchfox.org/firefox-main/rev/50445a0ee5bbeaeba68911b649963669e4c483e2/js/src/jit/shared/IonAssemblerBufferWithConstantPools.h#721 > [^2]: https://searchfox.org/firefox-main/rev/50445a0ee5bbeaeba68911b649963669e4c483e2/js/src/jit/shared/IonAssemblerBufferWithConstantPools.h#722-723 > [^3]: https://searchfox.org/firefox-main/rev/50445a0ee5bbeaeba68911b649963669e4c483e2/js/src/jit/shared/IonAssemblerBufferWithConstantPools.h#749-750 This is indeed correct! Good Catch! And for what is worth, this could potentially be leveraged on ARM64 as well. This issue is being addressed by [this patch](https://phabricator.services.mozilla.com/D270942) attached on this bug, and we should consider back-porting it to all branches on a release train of Firefox. Multiple branch sizes could be registered, such that one of the veneer (trampoline between out-of-reached regions) can be corrupted to encode an address which is incorrect, leading to a jump at an unexpected location on ARM64. [edit: remove the fact that this can be leveraged on ARM, given that ARM no longer seems to be using veneer]