erlang
compiler: Fix crash in beam_ssa_private_append
With the right input, the beam_ssa_private_append pass could crash when it, during the initial value tracking phase, selected literals which were not compatible with the tracked element structure for later patching. This patch ensures that only compatible literals are scheduled for later patching.
This crash is only present in OTP versions >= 26 && <= 27 and this fix should not be merged to master as dce585f933d0 ("compiler: In-place update of tuples/records"), merged for OTP 27, includes essentially the same functionality.
Closes #8630
CT Test Results
2 files 296 suites 10m 49s :stopwatch: 781 tests 779 :white_check_mark: 2 :zzz: 0 :x: 4 952 runs 4 950 :white_check_mark: 2 :zzz: 0 :x:
Results for commit 6b00105e.
:recycle: This comment has been updated with latest results.
To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.
See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.
Artifacts
// Erlang/OTP Github Action Bot
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@sverker, I don't have permissions to set labels, but shouldn't this get the "testing" label so it runs in nightly tests, even though its for maint-26?
maint-26 is currently locked as someone is making a 26 patch release. I don't even know if the testing label works for other branches than maint and master (maybe it does and I'm just ignorant). However, to get good testing on different machines, and as the branch has to be merged to maint anyway before releasing it on a maint-* branch, I (we) usually base the PR on maint.
So, I changed to PR to be on maint and set the testing label.
I can add it manually to the maint-26 testing as well, after the current release work is done, if you don't have access to our lab machines.
Ok, that did not work. It conflicted with 1f33624018df3e3df634e75ec787fcfba8ea9ccd which was merged into OTP-27.0 and is part of both maint and master.
I can put it into maint-26 testing when it opens up.
@IngelaAndin is planning a 26 patch tomorrow. So, if you (@frej) determine the risk/urgency ratio is low enough I could add it to maint-26 testing for tonight and to be released tomorrow.
I think that easiest way to handle patches is to, if possible base them on a green released version, and then most of the time the same branch can be run in both maint-XX track and maint (opu) and master builds to maximize test coverage. Sometimes it is of course not possible.
Thanks for the PR! I'll add it to testing once the patch is released, and merge to maint and master shortly afterwards.
@sverker: I don't know how the labels work, and I don't have access to any internal systems.
@sverker, @jhogberg and @IngelaAndin: as it says in the commit message, the patch can only go to the 26 branch, and should not be propagated to maint as it cannot be merged into master as, in master (and for that matter 27), the functionality this PR corrects has been superseded by a new better (and correct) implementation. I asked @bjorng how I should create the PR and he told me to base it on OTP-26.2 and target maint-26.
The bug was this PR corrects was discovered over a year after 26 was released and it can be avoided by just disabling the offending pass. The fix matches, in functionality if not in implementation, what's in 27, so I'd say its both low risk and low urgency.
@sverker, @jhogberg and @IngelaAndin: as it says in the commit message, the patch can only go to the 26 branch, and should not be propagated to maint as it cannot be merged into master as, in master (and for that matter 27), the functionality this PR corrects has been superseded by a new better (and correct) implementation. I asked @bjorng how I should create the PR and he told me to base it on OTP-26.2 and target maint-26.
Yes, you've done everything correctly. I'll take it from here :)