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

Add TLS support #28

Merged
merged 1 commit into from
Jun 4, 2024
Merged

Add TLS support #28

merged 1 commit into from
Jun 4, 2024

Conversation

0152la
Copy link
Contributor

@0152la 0152la commented Jun 3, 2024

Initial support for _Thread_local (and _Thread_local static) variables.

Limitations:

  • Supports a single TLS region, but amenable to future extensions if we want to implement multi-threaded compartments;
  • Handles only symbols of type TLSDESC.

Other changes:

  • Remove some old struct Compartment member variables that are now unneeded;
  • Add print_comp back;
  • Fix a bug regarding calculating scratch memory size for compartments;
  • Fixed a bug with setting relocation target addresses.

@0152la 0152la requested a review from ltratt June 3, 2024 11:40
include/compartment.h Outdated Show resolved Hide resolved
src/comp_utils.c Outdated Show resolved Hide resolved
if (to_map->libs[i]->tls_data_size != (size_t) -1)
{
assert(to_map->libs[i]->tls_sec_addr);
// TODO why 4?
Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed... Why 4?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I, uh, don't know. It seems to work with sizeof(void*), so I changed the question now to "Why sizeof(void*). This might be related to why printf doesn't work, but can't say without some in-depth debugging.

Copy link
Contributor Author

@0152la 0152la Jun 4, 2024

Choose a reason for hiding this comment

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

Alright, managed to uncover and fix an issue with subsequent library TLS regions (as I hinted) in f47b3c9. This was also part of that fix.

src/compartment.c Outdated Show resolved Hide resolved
@ltratt
Copy link
Contributor

ltratt commented Jun 4, 2024

I think this is ready for squashing?

@0152la
Copy link
Contributor Author

0152la commented Jun 4, 2024

Let me just double check the last unresolved comment, maybe I can figure out why it works with both 4 and sizeof(void*).

@ltratt
Copy link
Contributor

ltratt commented Jun 4, 2024

Please squash.

Initial support for `_Thread_local` (and `_Thread_local static`)
variables.

Limitations:
* Supports a single TLS region, but amenable to
future extensions if we want to implement multi-threaded compartments;
* Handles only symbols of type `TLSDESC`.

Other changes:
* Remove some old `struct Compartment` member variables that are now
  unneeded;
* Add `print_comp` back;
* Fix a bug regarding calculating scratch memory size for compartments;
* Fixed a bug with setting relocation target addresses.
@0152la 0152la force-pushed the actual-printf-not-2 branch from b2830ff to dacb4af Compare June 4, 2024 14:37
@0152la
Copy link
Contributor Author

0152la commented Jun 4, 2024

Squashed.

@ltratt ltratt added this pull request to the merge queue Jun 4, 2024
Merged via the queue into capablevms:master with commit ffc837d Jun 4, 2024
2 checks passed
@0152la 0152la deleted the actual-printf-not-2 branch June 4, 2024 14:45
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