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 support for more than two compartments #276

Closed
ayrtonm opened this issue Aug 28, 2023 · 4 comments · Fixed by #300
Closed

Add support for more than two compartments #276

ayrtonm opened this issue Aug 28, 2023 · 4 comments · Fixed by #300
Assignees
Labels

Comments

@ayrtonm
Copy link
Contributor

ayrtonm commented Aug 28, 2023

This is non-trivial because each call gate is specific to a pair of compartments. This means when compartment A and B both call foo in C we need separate call gates for them. Currently we insert call gates using ld --wrap which requires the interposed symbols to be globally unique so we'll have to rename these symbols (which can be done after compiling but before linking). See #174 for the preliminary plan.

@ayrtonm
Copy link
Contributor Author

ayrtonm commented Sep 13, 2023

Looks like add_custom_command PRE_LINK might make the objcopy --redefine-syms step mostly straighforward.

@ayrtonm
Copy link
Contributor Author

ayrtonm commented Sep 13, 2023

Also see $<TARGET_OBJECTS:some_test_target> for object files for each target, though I'm not sure how to best generate a custom command per object file/deal with objcopy's UI since the CMake var is a semi-colon separated list.

@ayrtonm
Copy link
Contributor Author

ayrtonm commented Sep 29, 2023

Here's a rough breakdown of the changes we need to make to implement this.

  • source rewriter (@ayrtonm)
    • check for functions that will need multiple call gates (i.e. in the FnDecl pass does a function appear in more than one set in std::set<Function> declared_fns[MAX_PKEYS] ignoring the set that defined it)
    • add _from_$CALLER_PKEY suffix to the direct call wrapper names (i.e. those that start with __wrap)
    • define an llvm::raw_fd_ostream *objcopy_redefine_syms_args[MAX_PKEYS] analogous to ld_args_out and lazily create these files like write_to_ld_file does. Only compartments that declare any of the suffixed functions need this file
    • For each suffixed function add a line with $FUNC $FUNC_from_$CALLER_PKEY to the corresponding objcopy args files
    • Change target function for these call gates
  • CMake (potentially @rinon)
    • add a function for defining a DSO in a separate compartment (can probably just modify define_shared_lib to allow invoking it more than once per test)
    • add the sources for the new compartment to the rewriter invocation in define-ia2-wrapper.cmake (this was already supported by ORIGINAL_TARGETS arg in define_ia2_wrapper`)
    • add a flag to define_shared_lib/define_test for if the DSO needs the objcopy file (analogous to NEEDS_LD_WRAP)
    • use add_custom_command with PRE_LINK to get cmake to run objcopy when the objcopy flag is set (see previous comment for getting the list of object files)
    • copy GENERATES_LD_WRAP logic in define-ia2-wrapper.cmake for the objcopy flag

@ayrtonm
Copy link
Contributor Author

ayrtonm commented Oct 10, 2023

I got a rough WIP working in PR #300. There are a few points where define_shared_lib/define_test need to be generalized for more than 3 pkeys and avoid assumptions about which DSOs are assigned which binaries Fixed the assumptions in define_test though it means that we can't infer a default value for its LIBS argument in the case that there is more than one compartment. There are probably ways around this but I'm not sure it's worth the extra complexity in the build system. Also the objcopy step currently only supports rewriting symbols in a DSO with a single .o More than one .o per DSO is now supported. @rinon the build system changes weren't as bad as I expected so I can finish these up myself.

The rewriter changes are done, but could use some cleanup/variable renaming to make things clearer I cleaned up the rewriter changes and added thorough comments so I just need to squash these commits. Also one commit just improves the error message when a compartment isn't defined so that should be split out into a separate PR.

For testing I modified read_config by renaming builtin.c to core.c and putting it in compartment 3. In retrospect, since plugins mainly work via indirect calls we should probably leave builtin.c in compartment 1 and create a another plugin .c for compartment 3. We should probably also add a test specifically for the new direct call gates (i.e. those that target renamed symbols) since the read_config test already has a lot and my changes for direct calls don't exercise that much functionality, but I'll wait until #286 is merged to redo the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants