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

Also strip Swift symbols when --objc_enable_binary_stripping is passed #13123

Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1567,7 +1567,7 @@ private void registerBinaryStripAction(Artifact binaryToLink, StrippingType stri
stripArgs = ImmutableList.of("-x");
break;
case DEFAULT:
stripArgs = ImmutableList.<String>of();
stripArgs = ImmutableList.of("-x", "-T");
Copy link
Member

Choose a reason for hiding this comment

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

kinda surprised -x wasn't here before

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, you guys sure -x belongs in the stripping of executables?

My impression was that one only wanted -x for dynamically loaded libraries (dylibs/kexts/loadable bundles) to preserve the global symbols needed to export an ABI from. From the strip man page, I'd guess you might want ("-u", "-r" instead for executables--and that that'd strip all the unneeded symbols?

Would you want to also strip Swift symbols in dynamically loaded libraries? (switch case immediately above) Apologies if I'm missing something--it's not clear to me what makes swift symbols special during the stripping process. Just ran across this while adding the -x case to fix loadable bundles in #13314

break;
default:
throw new IllegalArgumentException("Unsupported stripping type " + strippingType);
Expand Down