Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[vm] Bug fix in dedup optimization when build deferred components. #49393

Closed
wants to merge 1 commit into from
Closed

[vm] Bug fix in dedup optimization when build deferred components. #49393

wants to merge 1 commit into from

Conversation

lawangle
Copy link
Contributor

@lawangle lawangle commented Jul 4, 2022

Dedup optimization will remove duplicated Code object and keep one instance.
When building deferred components, the only Code object (e.x. as callee B) maybe
exist in different loading unit from the caller A. This will cause a rellocation
bug if A->B was a relative call.

Add a flag to indicate building with deferred components.
When working with dedup and deferred components, assignUnit first and
Code object from different unit is unequal.

Issues: #49372

Signed-off-by: fangzheyuan fangzheyuan@bytedance.com

@lawangle
Copy link
Contributor Author

lawangle commented Jul 4, 2022 •
edited
Loading

This PR fix a bug in building deferred component. I first found this error in #49372.
At first, i thought this bug only existed in older versions. But when i found out the real cause of the issue, i thought this bug still exists in current master branch.(Current master branch will crash in another issue: flutter/gallery#545 and this error will crash after 545's issue, so this error is covered)

I will explain this bug.
___TabsNonScrollableDemoState&State&SingleTickerProviderStateMixin&RestorationMixin._updateProperty) from Code([Optimized] __HomePageState&State&SingleTickerProviderStateMixin&RestorationMixin.registerForRestoration.listener)
This error log shows that xxx.registerForRestoration.listener(caller) called xxx._updateProperty(callee) but callee couldn't be resolved in relocation phase.

__HomePageState is in unit4 and ___TabsNonScrollableDemoState is in another unit and this call entry's kind is relative call, that means caller and callee shouid in the same unit. But they are in the different units, so crash.

The cause is the dedup optimization. Actually 2.16.2's dedup. In 2.16.2, the dedup try to merge Code Object instead of instruciton.
Before dedup, this call entry is from __HomePageState.xxx.registerForRestoration.listener to __HomePageState.xxx._updateProperty, relative call is ok.
After dedup, __HomePageState.xxx._updateProperty is replaced with ___TabsNonScrollableDemoState.xxx. _updateProperty and ___TabsNonScrollableDemoState is not exist in unit4 which cause relocation error.

@mraleph
Copy link
Member

mraleph commented Jul 4, 2022

Thanks for the contribution! Great catch.

Initial code review comments are here: https://dart-review.googlesource.com/c/sdk/+/250491

@lawangle lawangle force-pushed the fix_dedup_bug branch 3 times, most recently from 5764f54 to 4123acc Compare July 5, 2022 13:26
lawangle pushed a commit to lawangle/sdk that referenced this pull request Jul 5, 2022
@lawangle
[vm] Bug fix in dedup optimization when build deferred components.
711948c 
Closes dart-lang#49393

TEST=vm/dart{,_2}/regress_49372

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-product-x64-try
GitOrigin-RevId: c6306f9
Change-Id: Ie766b4c41fc61835767e62e3c463878e0493f812
lawangle pushed a commit to lawangle/sdk that referenced this pull request Jul 5, 2022
@lawangle
[vm] Bug fix in dedup optimization when build deferred components.
33d84ee 
Closes dart-lang#49393

TEST=vm/dart{,_2}/regress_49372

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-product-x64-try
GitOrigin-RevId: c6306f9
Change-Id: Ie766b4c41fc61835767e62e3c463878e0493f812
lawangle added a commit to lawangle/sdk that referenced this pull request Jul 5, 2022
@lawangle
[vm] Bug fix in dedup optimization when build deferred components.
6dc55ca 
Closes dart-lang#49393

TEST=vm/dart{,_2}/regress_49372

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-product-x64-try
GitOrigin-RevId: c6306f9
Change-Id: Ie766b4c41fc61835767e62e3c463878e0493f812

Signed-off-by: fzy <fangzheyuan@bytedance.com>
lawangle added a commit to lawangle/sdk that referenced this pull request Jul 5, 2022
@lawangle
[vm] Bug fix in dedup optimization when build deferred components.
624ee54 
Closes dart-lang#49393

TEST=vm/dart{,_2}/regress_49372

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-product-x64-try
GitOrigin-RevId: c6306f9
Change-Id: Ie766b4c41fc61835767e62e3c463878e0493f812

Signed-off-by: 方哲源 <fangzheyuan@bytedance.com>
@lawangle
[vm] Bug fix in dedup optimization when build deferred components.
29a2a77 
Closes dart-lang#49393

TEST=vm/dart{,_2}/regress_49372

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-product-x64-try
GitOrigin-RevId: c6306f9
Change-Id: Ie766b4c41fc61835767e62e3c463878e0493f812

Signed-off-by: 方哲源 <fangzheyuan@bytedance.com>
@lawangle
Copy link
Contributor Author

lawangle commented Jul 6, 2022

Change to https://dart-review.googlesource.com/c/sdk/+/250720/

@copybara-service copybara-service bot closed this in 6b04cac Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lawangle @mraleph