rewriter: Define call gates for function pointers in source defining the pointee #451
Conversation
ayrtonm
force-pushed
the
am/fvisibility
branch
3 times, most recently
from
0324836 to
3dcc745
Compare
|
Turns out we still need to distinguish between static and non-static address-taken functions even if they're both defined in a compartment .c because in the non-static case the call gate's target function may also be called directly from another compartment. When that happens the |
|
I also needed to distinguish between call gates for non-static address-taken functions in the same DSO and those for functions in another DSO. PR needs cleanup and updating comments but it should be working now. |
|
TODO: check |
ayrtonm
force-pushed
the
am/fvisibility
branch
from
30f4bfd to
2e7801e
Compare
…ource files defining the target These call gates were originally defined in the call gate .so for simplicity. Moving them to the source file defining the original address-taken function simplifies control-flow since it no longer requies cross-DSO calls and makes it possible to support programs that are intended to be compiled with -fvisibility=hidden since functions that may have hidden are only referenced by call gates within the DSO that defines them. Closes #435.
| /* Invoke the macro we just defined in the source file defining the target function */ | ||
| auto filename = fn_decl_pass.fn_definitions[fn_name]; | ||
| std::ofstream source_file(filename, std::ios::app); | ||
| source_file << "IA2_DEFINE_WRAPPER(" << fn_name << ")\n"; |
There was a problem hiding this comment.
Could we document how IA2_DEFINE_WRAPPER() and IA2_DEFINE_WRAPPER_* work somewhere?
There was a problem hiding this comment.
The comment was kind of misleading so I clarified it. This is one of those places where we're still trying to figure out what's the best way to interpose call gates as we get feedback from applying it to codebases so I don't think writing excessive documentation for things that may soon be out of date is a good use of our time.
There was a problem hiding this comment.
When I try this on dav1d, I'm able to successfully compile with -fvisibility=hidden again. The DEFINE_WRAPPER(...)s appear to be working. However, at runtime, I'm getting symbol lookup errors for these symbols:
dav1d_num_logical_processorsdav1d_init_cpudav1d_get_cpu_flags_x86
These symbols are all marked COLD (__attribute__((cold))), but when I tried removing this, the error persisted. I tried __attribute__((used)), too, which didn't work either. Adding DAV1D_API (__attribute__((visibility ("default")))) did work, but I'm not really sure why this is needed.
| @@ -920,6 +922,7 @@ class FnDecl : public RefactoringCallback { | |||
| std::set<Function> declared_fns[MAX_PKEYS]; | |||
| std::map<Function, AbiSignature> abi_signatures; | |||
| std::map<Function, Pkey> fn_pkeys; | |||
| std::map<Function, Filename> fn_definitions; | |||
There was a problem hiding this comment.
Is fn_definitions supposed to be sorted?
There was a problem hiding this comment.
it shouldn't need to be
| * define one call gate for each so we need to track them in case a function | ||
| * has its address taken in multiple places | ||
| */ | ||
| std::set<Function> generated_wrappers = {}; |
There was a problem hiding this comment.
Same for generated_wrappers; it doesn't need to be sorted, right?
There was a problem hiding this comment.
it shouldn't need to be
We previously defined call gates for pointers to static functions by appending a macro to the rewritten source files. In that case we had to do this because the static function may not be referenced from outside the translation unit. For pointers to globally-visible functions we defined the call gates in the call gate .so for simplicity. This commit changes the latter call gates so they're defined like the former. This change simplifies control-flow since the call gate no longer requires cross-DSO calls and makes it possible to support programs that are intended to be compiled with -fvisibility=hidden since functions that may have hidden visibility are only referenced by call gates within the DSO that defines them. Partially addresses #435.