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

Eager object symbol relocation #25

Merged
merged 1 commit into from
Mar 7, 2024
Merged

Conversation

0152la
Copy link
Contributor

@0152la 0152la commented Mar 5, 2024

Looking around in printf, we noticed it didn't work as objects were not being relocated across libraries. This PR enables this relocation; as well as adding a few extra tests.

TODOs:

  • we do not properly handle GLOBAL vs LOCAL relocations; it might be a small optimization to ensure we hold the two symbol types in different containers, so that when we do relocate, we don't needlessly look around at LOCAL bound symbols;
  • WEAK bound symbols aren't relocated;
  • some of the added tests are currently not fully workingm mainly due to (we believe) WEAK bound symbols, which are planned to be fixed shortly.

@0152la 0152la requested a review from ltratt March 5, 2024 15:31
// TODO at least right now
if (!strcmp(&dyn_str_tbl[curr_rela_sym.st_name], "environ"))
{
continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we print warnings for these, or will they fail in some nice determinstic way if someone uses them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's likely these will crash if used, although I haven't tested it. For now, I've added warnings to avoid using them in 13fcebd.

@ltratt
Copy link
Contributor

ltratt commented Mar 7, 2024

Please squash.

Looking around in `printf`, we noticed it didn't work as objects were
not being relocated across libraries. This PR enables this relocation;
as well as adding a few extra tests.

TODOs:
* we do not properly handle GLOBAL vs LOCAL relocations; it might be a
  small optimization to ensure we hold the two symbol types in different
  containers, so that when we do relocate, we don't needlessly look
  around at LOCAL bound symbols;
* WEAK bound symbols aren't relocated;
* some of the added tests are currently not fully workingm mainly due to
  (we believe) WEAK bound symbols, which are planned to be fixed
  shortly.
@0152la
Copy link
Contributor Author

0152la commented Mar 7, 2024

Squashed.

@ltratt ltratt added this pull request to the merge queue Mar 7, 2024
Merged via the queue into capablevms:master with commit e8bc807 Mar 7, 2024
2 checks passed
@0152la 0152la deleted the printf branch March 8, 2024 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants