Make sure all the relocations are filled in for partially cloned target #44262
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KristofferC
added
backport 1.6
33 tasks
50 tasks
vtjnash
approved these changes
Feb 19, 2022
47 tasks
We collect the relocations (i.e. the GOT slots that is used in the code) for each target
in `tgt.relocs`. Needing a relocation, however, does not imply that the function is cloned
for this target within the group (It does mean that at least one target
in the group has it cloned). The previous version would miss the relocation in this case.
This was triggerred with the following cloning situation
caller: clone_1
callee: clone_1, clone_1.clone_3
Since caller.clone_1 may call either callee.clone_1 or callee.clone_1.clone_3 a relocation
for callee will be used and is required to be initialized.
In addition to target 1, target 2 (and in fact target 3) within group 1
will also use caller.clone_1. However, since callee isn't cloned for target 2
the previous version wouldn't have saved this slot in the relocation array.
KristofferC
force-pushed
the
yyc/codegen/clone-reloc
branch
from
af178b2
to
2d932bb
Compare
staticfloat
added a commit
to JuliaCI/julia-buildkite-testing
that referenced
this issue
Mar 2, 2022
…et (JuliaLang#44262) We collect the relocations (i.e. the GOT slots that is used in the code) for each target in `tgt.relocs`. Needing a relocation, however, does not imply that the function is cloned for this target within the group (It does mean that at least one target in the group has it cloned). The previous version would miss the relocation in this case. This was triggerred with the following cloning situation caller: clone_1 callee: clone_1, clone_1.clone_3 Since caller.clone_1 may call either callee.clone_1 or callee.clone_1.clone_3 a relocation for callee will be used and is required to be initialized. In addition to target 1, target 2 (and in fact target 3) within group 1 will also use caller.clone_1. However, since callee isn't cloned for target 2 the previous version wouldn't have saved this slot in the relocation array.
KristofferC
added a commit
that referenced
this issue
Mar 3, 2022
…et (#44262) We collect the relocations (i.e. the GOT slots that is used in the code) for each target in `tgt.relocs`. Needing a relocation, however, does not imply that the function is cloned for this target within the group (It does mean that at least one target in the group has it cloned). The previous version would miss the relocation in this case. This was triggerred with the following cloning situation caller: clone_1 callee: clone_1, clone_1.clone_3 Since caller.clone_1 may call either callee.clone_1 or callee.clone_1.clone_3 a relocation for callee will be used and is required to be initialized. In addition to target 1, target 2 (and in fact target 3) within group 1 will also use caller.clone_1. However, since callee isn't cloned for target 2 the previous version wouldn't have saved this slot in the relocation array. (cherry picked from commit 76fc067)
KristofferC
added a commit
that referenced
this issue
Mar 7, 2022
…et (#44262) We collect the relocations (i.e. the GOT slots that is used in the code) for each target in `tgt.relocs`. Needing a relocation, however, does not imply that the function is cloned for this target within the group (It does mean that at least one target in the group has it cloned). The previous version would miss the relocation in this case. This was triggerred with the following cloning situation caller: clone_1 callee: clone_1, clone_1.clone_3 Since caller.clone_1 may call either callee.clone_1 or callee.clone_1.clone_3 a relocation for callee will be used and is required to be initialized. In addition to target 1, target 2 (and in fact target 3) within group 1 will also use caller.clone_1. However, since callee isn't cloned for target 2 the previous version wouldn't have saved this slot in the relocation array. (cherry picked from commit 76fc067)
LilithHafner
pushed a commit
to LilithHafner/julia
that referenced
this issue
Mar 8, 2022
…et (JuliaLang#44262) We collect the relocations (i.e. the GOT slots that is used in the code) for each target in `tgt.relocs`. Needing a relocation, however, does not imply that the function is cloned for this target within the group (It does mean that at least one target in the group has it cloned). The previous version would miss the relocation in this case. This was triggerred with the following cloning situation caller: clone_1 callee: clone_1, clone_1.clone_3 Since caller.clone_1 may call either callee.clone_1 or callee.clone_1.clone_3 a relocation for callee will be used and is required to be initialized. In addition to target 1, target 2 (and in fact target 3) within group 1 will also use caller.clone_1. However, since callee isn't cloned for target 2 the previous version wouldn't have saved this slot in the relocation array.
KristofferC
added a commit
that referenced
this issue
Mar 15, 2022
…et (#44262) We collect the relocations (i.e. the GOT slots that is used in the code) for each target in `tgt.relocs`. Needing a relocation, however, does not imply that the function is cloned for this target within the group (It does mean that at least one target in the group has it cloned). The previous version would miss the relocation in this case. This was triggerred with the following cloning situation caller: clone_1 callee: clone_1, clone_1.clone_3 Since caller.clone_1 may call either callee.clone_1 or callee.clone_1.clone_3 a relocation for callee will be used and is required to be initialized. In addition to target 1, target 2 (and in fact target 3) within group 1 will also use caller.clone_1. However, since callee isn't cloned for target 2 the previous version wouldn't have saved this slot in the relocation array. (cherry picked from commit 76fc067)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
We collect the relocations (i.e. the GOT slots that is used in the code) for each target
in
tgt.relocs. Needing a relocation, however, does not imply that the function is clonedfor this target within the group (It does mean that at least one target
in the group has it cloned). The previous version would miss the relocation in this case.
This was triggerred with the following cloning situation
Since caller.clone_1 may call either callee.clone_1 or callee.clone_1.clone_3 a relocation
for callee will be used and is required to be initialized.
In addition to target 1, target 2 (and in fact target 3) within group 1
will also use caller.clone_1. However, since callee isn't cloned for target 2
the previous version wouldn't have saved this slot in the relocation array.
AFAICT this bug has been here since the first merged version yet it only just triggered for me by 072c041 during precompilation of ColorTypes.jl in the compiler (caller was
cconvertfallback and callee was a no-opconvert.). This should probably be backported (and it should be able to backport cleanly all the way back to 0.7) even though apparently no one has hit it....