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

(WIP) rewriter: add __attribute__((visibility("default")) to IA2_FN address taken functions if they don't already default visibility #445

Closed
wants to merge 1 commit into from

Conversation

kkysen
Copy link
Contributor

@kkysen kkysen commented Oct 14, 2024

I tried copying the similar approach used to insert the __attribute__((used))s, and while its finding the right functions to add the __attribute__((visibility("default")))s to, the replacements are going in totally wrong places places, and I'm not sure what I'm doing wrong or quite how the replacements API is supposed to work. @ayrtonm or anyone else, do you have any suggestions?

…N` address taken functions if they don't already default visibility
@kkysen kkysen requested a review from ayrtonm October 14, 2024 11:23
@kkysen kkysen marked this pull request as draft October 14, 2024 11:23
@ayrtonm
Copy link
Contributor

ayrtonm commented Oct 14, 2024

Do we actually need fvisibility on the address taken functions? I think it's only the wrappers that need it.

@ayrtonm
Copy link
Contributor

ayrtonm commented Oct 14, 2024

Oh it's needed for non-static address taken functions. It might be better to define the call gates in the same .c like we do for static functions that way we don't have to prepend attributes to function definitions.

I'm guessing the issues you're running into probably stem from expansion/spelling loc differences. Doing these kinds of rewrites gets trickier when functions are defined in headers/expanded from macros so I'd like to avoid them if possible.

@ayrtonm
Copy link
Contributor

ayrtonm commented Oct 14, 2024

It might be better to define the call gates in the same .c like we do for static functions

I just realized that these may be referenced from multiple .c files so we need to either:

  1. arbitrarily pick a .c per compartment for non-static address-taken function call gates to put the call gate wrappers in
  2. or generate a .c per compartment and link that in

The first option is kinda ugly but I'm leaning towards that for expediency. @rinon thoughts?

@ayrtonm
Copy link
Contributor

ayrtonm commented Oct 14, 2024

For context I mostly want to avoid prepending attributes to functions with the rewriter to avoid cases like #414 where a function's definition has static or some attribute expanded from a macro and we can't reliably tell if the macro expansion includes anything else.

@ayrtonm
Copy link
Contributor

ayrtonm commented Oct 17, 2024

Superseded by #451.

@ayrtonm ayrtonm closed this Oct 17, 2024
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.

rewriter: support compiling with -fvisibility=hidden
2 participants