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.

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

When gen jal or jalr, retain some memory before pop BlockTrampolinePool. #139

Closed

Conversation

luyahan
Copy link
Collaborator

@luyahan luyahan commented Aug 11, 2020

RecordCallPosition record current pc into SafepointTable.
If pop a TrampolinePool after jalr, return address in stack cant't be found in SafepointTable.
Fix #56.
Fix #129

@luyahan luyahan force-pushed the fix-56-unreachable-code branch 2 times, most recently from 357830c to 4587936 Compare August 11, 2020 06:22
@luyahan luyahan changed the title Avoid pop a BlockTrampolinePool before Call and RecordCallPosition. Avoid pop a BlockTrampolinePool between Call and RecordCallPosition. Aug 11, 2020
@luyahan luyahan added this to the native-build (09/04/2020) milestone Aug 11, 2020
@luyahan luyahan changed the title Avoid pop a BlockTrampolinePool between Call and RecordCallPosition. When gen jal or jalr, retain some memory before pop BlockTrampolinePool. Aug 11, 2020
@penguinwu penguinwu requested review from penguinwu and a user August 11, 2020 14:28
@penguinwu
Copy link

Great work @luyahan!

Could you give an example of the problem the is prevented by the fix?

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Nice discovery!

@luyahan
Copy link
Collaborator Author

luyahan commented Aug 12, 2020

Great work @luyahan!

Could you give an example of the problem the is prevented by the fix?

In code-generator-riscv64.cc:615, it call RecordCallPosition to record the pc after Call into SafepointTable. But if Call pop a BlockTrampolinePool like it :

                  [ kArchCallCodeObject
                  -- Inlined Trampoline to CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit --
0x9be7ec53f0   f30  00000fb7       lui       t6, 0x0         ;; off heap target
0x9be7ec53f4   f34  7f6f8f9b       addiw     t6, t6, 2038
0x9be7ec53f8   f38  00cf9f93       slli      t6, t6, 12
0x9be7ec53fc   f3c  c9af8f93       addi      t6, t6, -870
0x9be7ec5400   f40  00cf9f93       slli      t6, t6, 12
0x9be7ec5404   f44  8a9f8f93       addi      t6, t6, -1879
0x9be7ec5408   f48  00cf9f93       slli      t6, t6, 12
0x9be7ec540c   f4c  520f8f93       addi      t6, t6, 1312
0x9be7ec5410   f50  000f80e7       jalr      t6
0x9be7ec5414   f54  0840006f       j         132 -> 0x9be7ec5498  <+0xfd8>
0x9be7ec5418   f58  7090006f       j         3848 -> 0x9be7ec6320  <+0x1e60>
0x9be7ec541c   f5c  72d0006f       j         3884 -> 0x9be7ec6348  <+0x1e88>
0x9be7ec5420   f60  7ad0006f       j         4012 -> 0x9be7ec63cc  <+0x1f0c>
0x9be7ec5424   f64  7c90006f       j         4040 -> 0x9be7ec63ec  <+0x1f2c>
0x9be7ec5428   f68  7e10006f       j         4064 -> 0x9be7ec6408  <+0x1f48>
0x9be7ec542c   f6c  7ed0006f       j         4076 -> 0x9be7ec6418  <+0x1f58>
0x9be7ec5430   f70  6900106f       j         5776 -> 0x9be7ec6ac0  <+0x2600>
0x9be7ec5434   f74  5050106f       j         7428 -> 0x9be7ec7138  <+0x2c78>
0x9be7ec5438   f78  5290106f       j         7464 -> 0x9be7ec7160  <+0x2ca0>
0x9be7ec543c   f7c  54d0106f       j         7500 -> 0x9be7ec7188  <+0x2cc8>
0x9be7ec5440   f80  5710106f       j         7536 -> 0x9be7ec71b0  <+0x2cf0>
0x9be7ec5444   f84  5950106f       j         7572 -> 0x9be7ec71d8  <+0x2d18>
0x9be7ec5448   f88  5b90106f       j         7608 -> 0x9be7ec7200  <+0x2d40>
0x9be7ec544c   f8c  5dd0106f       j         7644 -> 0x9be7ec7228  <+0x2d68>
0x9be7ec5450   f90  6010106f       j         7680 -> 0x9be7ec7250  <+0x2d90>
0x9be7ec5454   f94  6250106f       j         7716 -> 0x9be7ec7278  <+0x2db8>
0x9be7ec5458   f98  6490106f       j         7752 -> 0x9be7ec72a0  <+0x2de0>
0x9be7ec545c   f9c  66d0106f       j         7788 -> 0x9be7ec72c8  <+0x2e08>
0x9be7ec5460   fa0  6910106f       j         7824 -> 0x9be7ec72f0  <+0x2e30>
0x9be7ec5464   fa4  7050106f       j         7940 -> 0x9be7ec7368  <+0x2ea8>
0x9be7ec5468   fa8  7290106f       j         7976 -> 0x9be7ec7390  <+0x2ed0>
0x9be7ec546c   fac  74d0106f       j         8012 -> 0x9be7ec73b8  <+0x2ef8>
0x9be7ec5470   fb0  7710106f       j         8048 -> 0x9be7ec73e0  <+0x2f20>
0x9be7ec5474   fb4  7950106f       j         8084 -> 0x9be7ec7408  <+0x2f48>
0x9be7ec5478   fb8  7b90106f       j         8120 -> 0x9be7ec7430  <+0x2f70>
0x9be7ec547c   fbc  7dd0106f       j         8156 -> 0x9be7ec7458  <+0x2f98>
0x9be7ec5480   fc0  0000206f       j         8192 -> 0x9be7ec7480  <+0x2fc0>
0x9be7ec5484   fc4  0240206f       j         8228 -> 0x9be7ec74a8  <+0x2fe8>
0x9be7ec5488   fc8  0480206f       j         8264 -> 0x9be7ec74d0  <+0x3010>
0x9be7ec548c   fcc  06c0206f       j         8300 -> 0x9be7ec74f8  <+0x3038>
0x9be7ec5490   fd0  0900206f       j         8336 -> 0x9be7ec7520  <+0x3060>
0x9be7ec5494   fd4  0040006f       j         4 -> 0x9be7ec5498  <+0xfd8>
                  ]
0x9be7ec4bd8   fd8  fff00893       li        a7, -1

so record 0x9be7ec4bd8 not jalr return address 0x9be7ec5414 into safepoint tale.

This patch add a remain address after Call. It gen code like:

0x9be7ec4b34   f34  7f4f8f9b       addiw     t6, t6, 2036
0x9be7ec4b38   f38  00cf9f93       slli      t6, t6, 12
0x9be7ec4b3c   f3c  b64f8f93       addi      t6, t6, -1180
0x9be7ec4b40   f40  00cf9f93       slli      t6, t6, 12
0x9be7ec4b44   f44  b5ef8f93       addi      t6, t6, -1186
0x9be7ec4b48   f48  00cf9f93       slli      t6, t6, 12
0x9be7ec4b4c   f4c  4e0f8f93       addi      t6, t6, 1248
0x9be7ec4b50   f50  000f80e7       jalr      t6
0x9be7ec4b54   f54  fff00893       li        a7, -1
0x9be7ec4b58   f58  0840006f       j         132 -> 0x9be7ec4bdc  <+0xfdc>
0x9be7ec4b5c   f5c  7050006f       j         3844 -> 0x9be7ec5a60  <+0x1e60>
0x9be7ec4b60   f60  7290006f       j         3880 -> 0x9be7ec5a88  <+0x1e88>
0x9be7ec4b64   f64  7a90006f       j         4008 -> 0x9be7ec5b0c  <+0x1f0c>
0x9be7ec4b68   f68  7c50006f       j         4036 -> 0x9be7ec5b2c  <+0x1f2c>
0x9be7ec4b6c   f6c  7dd0006f       j         4060 -> 0x9be7ec5b48  <+0x1f48>
0x9be7ec4b70   f70  7e90006f       j         4072 -> 0x9be7ec5b58  <+0x1f58>
0x9be7ec4b74   f74  68c0106f       j         5772 -> 0x9be7ec6200  <+0x2600>
0x9be7ec4b78   f78  5010106f       j         7424 -> 0x9be7ec6878  <+0x2c78>
0x9be7ec4b7c   f7c  5250106f       j         7460 -> 0x9be7ec68a0  <+0x2ca0>
0x9be7ec4b80   f80  5490106f       j         7496 -> 0x9be7ec68c8  <+0x2cc8>
0x9be7ec4b84   f84  56d0106f       j         7532 -> 0x9be7ec68f0  <+0x2cf0>
0x9be7ec4b88   f88  5910106f       j         7568 -> 0x9be7ec6918  <+0x2d18>
0x9be7ec4b8c   f8c  5b50106f       j         7604 -> 0x9be7ec6940  <+0x2d40>
0x9be7ec4b90   f90  5d90106f       j         7640 -> 0x9be7ec6968  <+0x2d68>
0x9be7ec4b94   f94  5fd0106f       j         7676 -> 0x9be7ec6990  <+0x2d90>
0x9be7ec4b98   f98  6210106f       j         7712 -> 0x9be7ec69b8  <+0x2db8>
0x9be7ec4b9c   f9c  6450106f       j         7748 -> 0x9be7ec69e0  <+0x2de0>
0x9be7ec4ba0   fa0  6690106f       j         7784 -> 0x9be7ec6a08  <+0x2e08>
0x9be7ec4ba4   fa4  68d0106f       j         7820 -> 0x9be7ec6a30  <+0x2e30>
0x9be7ec4ba8   fa8  7010106f       j         7936 -> 0x9be7ec6aa8  <+0x2ea8>
0x9be7ec4bac   fac  7250106f       j         7972 -> 0x9be7ec6ad0  <+0x2ed0>
0x9be7ec4bb0   fb0  7490106f       j         8008 -> 0x9be7ec6af8  <+0x2ef8>
0x9be7ec4bb4   fb4  76d0106f       j         8044 -> 0x9be7ec6b20  <+0x2f20>
0x9be7ec4bb8   fb8  7910106f       j         8080 -> 0x9be7ec6b48  <+0x2f48>
0x9be7ec4bbc   fbc  7b50106f       j         8116 -> 0x9be7ec6b70  <+0x2f70>
0x9be7ec4bc0   fc0  7d90106f       j         8152 -> 0x9be7ec6b98  <+0x2f98>
0x9be7ec4bc4   fc4  7fd0106f       j         8188 -> 0x9be7ec6bc0  <+0x2fc0>
0x9be7ec4bc8   fc8  0200206f       j         8224 -> 0x9be7ec6be8  <+0x2fe8>
0x9be7ec4bcc   fcc  0440206f       j         8260 -> 0x9be7ec6c10  <+0x3010>
0x9be7ec4bd0   fd0  0680206f       j         8296 -> 0x9be7ec6c38  <+0x3038>
0x9be7ec4bd4   fd4  08c0206f       j         8332 -> 0x9be7ec6c60  <+0x3060>
0x9be7ec4bd8   fd8  0040006f       j         4 -> 0x9be7ec4bdc  <+0xfdc>
0x9be7ec4bdc   fdc  ff010113       addi      sp, sp, -16

It will reocord 0x9be7ec4b54 into safepoint table.

@ghost
Copy link

ghost commented Aug 12, 2020

I rebased this and reopened as #144.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants