[llvm-objcopy][MachO] Add support for removing Swift symbols

[alexander-shaposhnikov]

Summary

cctools' strip has the option "-T" which removes the symbols whose names begin with _$S' or _$s' only if it finds an __objc_image_info section containing a non-zero swift version (and the binary is either an executable or a dynamic library).
This diff implements this option in llvm-strip for MachO.

Test plan: make check-all

[alexander-shaposhnikov]

Add early return

[alexander-shaposhnikov]

Apply clang-format

[smeenai]

The logic looks good and matches up with cctools strip.

The man page does say:

-T     The intent of this flag is to remove Swift symbols.  It removes the symbols whose names begin with `_$S' or `_$s' only when it finds an __objc_imageinfo section with and it has a non-zero swift version.  The future the implementation of this  flag
       may change to match the intent.

so we should keep an eye out for future behavioral changes. Also CC @mtrent if he has any input on that.

llvm/test/tools/llvm-objcopy/MachO/remove-swift-symbols.test

2

Can you also add a test for Swift symbols *not* being stripped when a Swift version isn't present? It would also be great to test _$S symbols in addition to _$s ones.

llvm/tools/llvm-objcopy/COFF/COFFObjcopy.cpp

255

This is okay for now, but note that Swift supports both COFF and ELF, so theoretically this flag could apply there as well. I don't know what the equivalent symbols would look like on COFF or ELF though ... @compnerd?

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

607–609

Same coment here.

llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp

77

& has stupidly low precedence in C/C++ (because && didn't exist once upon a time), so this is gonna parse as

(Obj.Header.Flags & (MachO::MH_DYLDLINK == MachO::MH_DYLDLINK))

You'll need the parens around (Obj.Header.Flags & MachO::MH_DYLDLINK) instead.

Moreover, since MH_DYLDLINK is a single bit, can't you just do

(Obj.Header.Flags & MachO::MH_DYLDLINK)

or, if you wanna be really explicit

(Obj.Header.Flags & MachO::MH_DYLDLINK) != 0

?

[alexander-shaposhnikov]

llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp

77

good catch, thanks!

[alexander-shaposhnikov]

Address comments

[smeenai]

LGTM

[alexander-shaposhnikov]

Apply clang-format

[jhenderson]

Please don't forget to update the CommandGuide doc for llvm-strip. Is this option deliberately not an llvm-objcopy option?

llvm/test/tools/llvm-objcopy/MachO/remove-swift-symbols.test

7

Nit: it's fairly typical to ad addditional spaces to makes the first line here line up with the rest of the output:

# NO-SWIFT-SYMBOLS:      Symbols [
# NO-SWIFT-SYMBOLS-NEXT:   Symbol {

Same applies below.

136

Probably change "since" -> "when"

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

608

I think there's a std::errc::not_supported you could use here.

llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp

76–77

Seems like you need test cases here for a) MH_DYLDLINK not being set and b) SwiftVersion being 0.

I personally would find it easier to read if the version check were explicitly compared against 0: *Obj.SwitftVersion != 0.

llvm/tools/llvm-objcopy/MachO/MachOReader.cpp

294–297

Seems like most of these clauses in this if are untested? I'd expect to see various test inputs with an __objc_imageinfo in each of these segments, plus one with it being in a different segment and one that is too small.

299

This also seems to be untested.

[alexander-shaposhnikov]

llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp

76–77

@jhenderson :

1. If you look at the man pages for cctools' strip then you will see that there are remarks there indicating that this behavior can change in the future. Regarding tests (what you mentioned in the comment above and below) - I didn't do that intentionally since it looks like they will have to be either removed or adjusted in the future, the current implementation is consistent with the latest version of cctools.

If you insist on covering those (very similar) cases - we can add them, though I think right now it seems to be ahead of time / not very useful.

2. I'm not aware of any official objcopy (from Apple) which would work for MachO (in particular, cctools contains strip and other things, but not objcopy), so right now it looks like there is no need to introduce such flag to llvm-objcopy. If somebody requests it - sure, we can consider adding it. However, since we are trying to make llvm-strip a drop-in replacement for strip (in this particular case - I'm referring to the tool 'strip' from Xcode (cctools)) - we have to implement this option.

3. regarding the comment about big-endian - if I'm not mistaken the PowerPC backend (for Apple platforms) was removed even from Clang a few years ago, so at the moment it's a bit unrealistic (if I'm not mistaken) that we'll have a big-endian MachO executable or dynamic library built from Swift sources. However, since people can use LLVM on a big-endian host machine (and if I am not mistaken we have such buildbots) - we need to take care of those conversions. If there is an issue those big-endian buildbots are supposed to catch it.

[alexander-shaposhnikov]

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

608

I think it's important to be consistent in what we return in such cases,
I've looked at the COFF / MachO versions - they both return invalid_argument.
So my point is that if we want to change it then it should be discussed and done separately.

[alexander-shaposhnikov]

llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp

76–77

Looks like I have an idea how to organize these tests in a better way, I'll update this diff.

[alexander-shaposhnikov]

Address @jhenderson's comments

[jhenderson]

Thanks @alexshap. The only thing outstanding from my point of view, aside from a nit, is documenting the switch. I think we can keep it concise in llvm-strip's documentation by just saying the same thing as the help text. That way, if details change, it's not a problem. Users probably don't need to know the details. BTW, I couldn't quickly find a cctools strip man page online, so I can't comment on their documentation.

llvm/test/tools/llvm-objcopy/MachO/remove-swift-symbols.test

1

Here and below: swift -> Swift

[alexander-shaposhnikov]

llvm/test/tools/llvm-objcopy/MachO/remove-swift-symbols.test

1

yeah )

[alexander-shaposhnikov]

Update CommandGuide/llvm-strip.rst

[jyknight]

Isn't this potentially problematic from a compatibility point-of-view, since it squats a single-letter command-line argument for a mach-o-specific feature, while llvm-strip is generally trying to be compatible to GNU strip?

[MaskRay]

In D80099#2056447, @jyknight wrote:

Isn't this potentially problematic from a compatibility point-of-view, since it squats a single-letter command-line argument for a mach-o-specific feature, while llvm-strip is generally trying to be compatible to GNU strip?

I think it may be problematic if GNU strip reserves -T for other purposes. Can we have a --strip-* long option instead? --strip_swift_symbols (the obvious option name inferred from the TableGen variable) may not be most suitable because from the description "_$S' or _$s' only if it finds an __objc_image_info section containin" this option only strips some special cased symbols, not all Swift symbols.

[alexander-shaposhnikov]

Thank you for the question!

First, I checked in advance the documentation https://sourceware.org/binutils/docs/binutils/strip.html - there is no conflict for this option, if one day this issue comes up, or, more generally, if one day we have to parse command-line options differently for different formats (in this case it might be MachO or COFF or ELF) then the approach used in llvm-objcopy/llvm-strip will have to be adjusted anyway. It looks like at the moment there is no urgency to rename this option (+ it would introduce some friction for users on OSX, they won't be able to seamlessly switch from cctools strip to llvm-strip, especially for large complex projects). Additionally, If these issues arise, I'm not sure renaming is really the right solution, although this question requires more thinking (i'm not ready to answer it right away). One can imagine several other options how to resolve this hypothetical issue without breaking the downstream users (or at least with much smaller disruption): either introduce a "new driver", or (similarly to llvm-objdump) introduce "--macho", there are other approaches as well (it's not the full list).

Second, to the best of my knowledge the support for MachO in binutils is fragmented (e.g. strip doesn't properly support MachO if I'm not mistaken) and even small bits which exist are outdated (e.g. after a very brief look I don't see any mentions of the load commands which were introduced in the iOS era).

For Apple platforms (iOS, OSX) it's common to use tools provided by Xcode, and (again, to the best of my knowledge) several important utilities (distributed by Xcode) (install-name-tool, strip, lipo, bitcode-strip) actually come from cctools.
So to use llvm-strip in place of cctools' strip we need functional parity (at least to some extent). Moreover, having support for these features is also useful in some other cases. For example, in the past I used to work on Swift,
there are developers hacking on Swift which are using non-Apple platforms as their primary development environments, having such tools unblocks a number of use-cases
(the most trivial one - running tests for MachO binaries (which don't require the actual runtime, e.g. various tools which parse / extract some information from object files) on non-OSX host platforms).

[jhenderson]

I'm okay with using the short option (although a long option wouldn't hurt too FWIW). I think it's useful to have compatibility with as many strip tools as we reasonably can. Whilst GNU compatibility is obviously our primary concern, where GNU doesn't have certain options but other versions of strip do, I think we should emulate them. The benefits have already been outlined (less friction in transitioning etc). A good example is the --strip-sections option which I believe came from eu-strip. I would hope that GNU might consider what options other tools already provide when choosing new options, so I would hope that risk of future compatibility is reduced. When there is no precedent for a short option in other strip/objcopy tools however, I think it is highly desirable to not add a short option alias ourselves, unless the meaning is obvious (e.g. in GNU dumping tools, -C always means --demangle, so if we have --demangle, having -C is fine even though GNU don't currently).

[jyknight]

We already _aren't_ and _cannot_ be compatible with cctools strip's command-line, because cctools strip already has a bunch of one-letter option choices which actively conflict with existing options in llvm-strip (and GNU strip from which we copied them). There's at least -s -R -d -N with conflicting meanings.

Given that, I do not think adding "-T" to llvm-strip is good idea. While it may not conflict today, it might tomorrow. With command-line compatibility to cctools out of the question, it then comes down to a question of naming -- and "-T" is just a bad choice for a special-purpose option like this. It ought to have been given a descriptive option name in the first place.

I do think we should just rename this to --strip-swift-symbols for llvm-strip. If, in the future, someone wants to make another command which is a drop-in-replacement for cctools strip, that could be done, and it will need to do option-translation.

[alexander-shaposhnikov]

@jyknight, thank you for the comment.

1. Just to be clear - introducing new names alone will make it incompatible with a huge variety of existing projects / their build systems. If hypothetically you move this burden to the layer of a build system then users will have to deal with this issue N times, where N is equal to the number of users or to the number of different build systems (depending on how you count). It would create an unnecessary obstacle and would cause massive duplication of efforts.

P.S. llvm-objcopy indeed has reimplemented a number of command-line options of binutils objcopy,
but it also has some extensions (flags which binutils objcopy doesn't support) as well as some omissions (flags which binutils objcopy supports but llvm-objcopy doesn't) so in this respect it doesn't create a new precedent.
It's also true for other binary-level tools in LLVM which emulate (to some extent) the interface of their counterparts from other projects.

2. The current state of llvm-strip - some projects build successfully if one switches to llvm-strip, though certainly these tools are still under development (and this statement is not specific to MachO) and different features are implemented when they are needed. I'm aware of several ways how people use llvm-objcopy / llvm-strip for MachO.

That said, your concern is valid, indeed, in the future the issue which you mentioned may come up (though there is no conflict now, moreover, introducing a new name would break the existing use-cases).
Given that bintuils strip for MachO is not used (and it's not really fully functional if I am not mistaken) - the most common (and sort of the most realistic) way to use llvm-strip for MachO is to use it in place of the "system strip" (coming cctools) (on OSX).

Taking into account what was mentioned in (1) renaming (or renaming alone) doesn't appear to the right solution.

As soon as we run into this issue - no doubt, a solution will need to be implemented. The current state doesn't appear to block it or prevent it in any way, moreover, it enables us to move step by step
and there are multiple ways to solve this problem.
"If, in the future, someone wants to make another command which is a drop-in-replacement for cctools strip, that could be done, and it will need to do option-translation" - this is essentially one of the options mentioned above, though the details require more thinking.
Let me point out that llvm-strip is also just another driver for llvm-objcopy (it was introduced here: https://reviews.llvm.org/rL331663),
all what it does - it translates options and invokes llvm-objcopy).

[MaskRay]

Reading https://opensource.apple.com/source/cctools/cctools-949.0.1/man/strip.1.auto.html , I confirm that -S -r -D -n are conflicting.

In D80099#2057816, @alexshap wrote:

@jyknight, thank you for the comment.

1. Just to be clear - introducing new names alone will make it incompatible with a huge variety of existing projects / their build systems. If hypothetically you move this burden to the layer of a build system then users will have to deal with this issue N times, where N is equal to the number of users or to the number of different build systems (depending on how you count). It would create an unnecessary obstacle and would cause massive duplication of efforts.

How can llvm-strip be plugged into an existing project if -S -r -D -n (supposedly common usage) do not work?

P.S. llvm-objcopy indeed has reimplemented a number of command-line options of binutils objcopy,
but it also has some extensions (flags which binutils objcopy doesn't support) as well as some omissions (flags which binutils objcopy supports but llvm-objcopy doesn't) so in this respect it doesn't create a new precedent.
It's also true for other binary-level tools in LLVM which emulate (to some extent) the interface of their counterparts from other projects.

FWIW For many such extensions I have created feature requests on binutils bugzilla or even implemented the features by myself.
If we are going to add a new long option, I hope we can see efforts asking cctools to adopt. Some option names may potentially be reused with different semantics. If they don't want to implement those features, we can at least get a promise that they will not intentionally use the options names for difference purposes.

For this particular case, -T is a really unfortunate name for a special cased feature. I think we need to persuade them to add a long option.

2. The current state of llvm-strip - some projects build successfully if one switches to llvm-strip, though certainly these tools are still under development (and this statement is not specific to MachO) and different features are implemented when they are needed. I'm aware of several ways how people use llvm-objcopy / llvm-strip for MachO.

That said, your concern is valid, indeed, in the future the issue which you mentioned may come up (though there is no conflict now, moreover, introducing a new name would break the existing use-cases).
Given that bintuils strip for MachO is not used (and it's not really fully functional if I am not mistaken) - the most common (and sort of the most realistic) way to use llvm-strip for MachO is to use it in place of the "system strip" (coming cctools) (on OSX).

Given the -S -r -D -n incompatibility, I think we will run into trouble soon.

[alexander-shaposhnikov]

@MaskRay - if i'm not mistaken for dynamic libraries the maximum level of stripping is usually -x (or -x -T), also for executables it's frequently used without any options at all (e.g. strip main.exe).
Also note that we reached a functional state for some non-trivial transformations (in llvm-objcopy) relatively recently (~weeks ago from now) .
Yeah, given the discrepancies it looks like we'll have to solve this problem soon.

[jyknight]

The current state doesn't appear to block it or prevent it in any way

It does prevent it. This change introduces an argument, -T to the option syntax of the llvm-strip tool, which we'd be better off without. But, once we add it, users of llvm-strip will depend on it, and then we can't easily remove it later on. The only argument for the name -T is compatibility with cctools strip, which is unachievable within a single driver syntax, anyways.

Now, if it wasn't for all the other incompatibilities with cctools' strip, I could buy the argument that it's worth adding -T, despite that it was a bad choice of name, and despite the potential for future conflicts. But, since compatibility is impossible, we have those downsides, and little upside to go along with it.

introducing a new name would break the existing use-cases

I don't see how, since llvm-strip isn't a drop in replacement for cctools strip anyhow.

[alexander-shaposhnikov]

In D80099#2058039, @jyknight wrote:

The current state doesn't appear to block it or prevent it in any way

It does prevent it. This change introduces an argument, -T to the option syntax of the llvm-strip tool, which we'd be better off without. But, once we add it, users of llvm-strip will depend on it, and then we can't easily remove it later on. The only argument for the name -T is compatibility with cctools strip, which is unachievable within a single driver syntax, anyways.

@jyknight - alright, I wouldn't like to repeat the arguments already presented and want to put this discussion into a more constructive course to avoid moving back and forth.

"It does not" for the following reasons. First, it's supported only for MachO (and by the way there are options supported only for ELF), so the only way users can depend on it is a legitimate one (the system strip supports this flag and various projects use it). Second, from users' perspective the fact that "strip" is only a driver for another tool (whether it's the existing driver, the existing driver with a new "flavor" or a new driver) is a detail of implementation (and it is hidden from them). So if e.g. we introduce a new way to invoke llvm-strip (llvm-objcopy) it should not affect them / disrupt their workflow, thus from users' perspective it doesn't fundamentally alter this change.
Also, please, note that the argument regarding binutils for MachO is invalid (see comments above).

Now, if it wasn't for all the other incompatibilities with cctools' strip, I could buy the argument that it's worth adding -T, despite that it was a bad choice of name, and despite the potential for future conflicts. But, since compatibility is impossible, we have those downsides, and little upside to go along with it.

introducing a new name would break the existing use-cases

I don't see how, since llvm-strip isn't a drop in replacement for cctools strip anyhow.

Let me remind (and it has already been mentioned above) it's not fully functional yet but it's moving forward, and there are projects which build with it, it's just one of the first steps,
introducing a new name would break it without providing much benefits at all.
Basically llvm-strip would exist on OSX but would be unusable and would make it harder even to test it / plug into a real world project and see what features are missing and what needs to be fixed.
Based on the experience with ELF I'm pretty sure it's a long path and we will encounter and fix issues along the way, and llvm-objcopy (llvm-strip) for ELF didn't magically become compatible with binutils immediately.

That said, the problem with the discrepancies will have to be addressed at some point, nobody disagrees with it and, in particular, I'm open to discuss the approaches, should it be a new driver, a new "flavor" or something else.

[jhenderson]

I don't have a good solution to the conflict here, but I want to summarise some of the options we have, with a brief summary of the pros and cons of each. This is duplicating some of what is above, but will hopefully help frame my own and possibly others thinking:

1. Add a long option instead of a short option. Pros: simple, shouldn't be incompatible with a future GNU option abbreviation. Cons: Incompatible with cctools strip, making it impossible to just drop it in as a replacement for that tool for usages that otherwise would work (i.e. which don't use other incompatible options).
2. Add a long option and a short option. Pros: provides a migration path for users who want to adopt llvm-strip instead of cctools trip; provides a possible future alternative in the event that GNU use -T for something else. We'd probably want to remove our alias, which would break some people's builds, but at least they'd have an alternative without changing away from using llvm-strip. Cons: people might start using -T when they should use the long option. This could be somewhat mitigated with documentation/warnings etc, but perhaps this is suboptimal. Also doesn't allow people to smoothly migrate if we have to remove the short option in the future. Also we'd have to have some sort of delay between removing the short option and reinstating it with the new meaning to prevent a silent behaviour change. This throws up questions like "how long a delay", so probably isn't ideal.
3. Have a different cctools-compatible front-end for llvm-strip. Pros: allows users of cctools strip to migrate. Cons: requires more maintenance (more tablegen options etc to maintain); wouldn't work as a system linker using the current determined-by-filename approach (currently the file name drives whether the tool is llvm-objcopy or llvm-strip and we already say "if name contains strip, treat as llvm-strip", so can't overload it with "treat as cctools strip").
4. Have a different option (e.g. --macho) that controls the behaviour of option parsing. Pros: allows us to treat options differently depending on the presence of the switch. Cons: doesn't allow for immediate drop-in replacement.
5. New idea: change behaviour based on host OS, similar to llvm-ar behaviour and llvm-cxxfilt --strip-underscore. Pros: allows drop-in replacement on Mach-O based systems; has some precendent in the other tools. Cons: doesn't allow for cross testing easily (i.e. would require something like --macho too); could be a little confusing to future users.

I just came up with it, but I like 5). I think it would satisfy most, possibly all of our requirements, has existing precedent in our other tools, and poses a low risk of future incompatibility. What do others think?

llvm/test/tools/llvm-objcopy/MachO/remove-swift-symbols.test

+## Verify that -T removes swift symbols.
+# RUN: yaml2obj -D FLAGS=0x00200085 -D SEGMENT_NAME=__DATA \
+# RUN:  -D SECTION_NAME=__objc_imageinfo -D SECTION_SIZE=8 \
+# RUN:  -D SECTION_CONTENT=0000000040070105 %s -o %t1
+# RUN: llvm-strip -x -T %t1
+# RUN: llvm-readobj -symbols %t1 | FileCheck --check-prefix=NO-SWIFT-SYMBOLS %s
+
+# RUN: yaml2obj -D FLAGS=0x00200085 -D SEGMENT_NAME=__DATA_CONST \
+# RUN:  -D SECTION_NAME=__objc_imageinfo -D SECTION_SIZE=8 \
+# RUN:  -D SECTION_CONTENT=0000000040070105 %s -o %t2
+# RUN: llvm-strip -x -T %t2
+# RUN: llvm-readobj -symbols %t2 | FileCheck --check-prefix=NO-SWIFT-SYMBOLS %s
+
+# RUN: yaml2obj -D FLAGS=0x00200085 -D SEGMENT_NAME=__DATA_DIRTY \
+# RUN:  -D SECTION_NAME=__objc_imageinfo -D SECTION_SIZE=8 \
+# RUN:  -D SECTION_CONTENT=0000000040070105 %s -o %t3
+# RUN: llvm-strip -x -T %t3
+# RUN: llvm-readobj -symbols %t3 | FileCheck --check-prefix=NO-SWIFT-SYMBOLS %s
+
+# NO-SWIFT-SYMBOLS:      Symbols [
+# NO-SWIFT-SYMBOLS-NEXT:  Symbol {
+# NO-SWIFT-SYMBOLS-NEXT:    Name: _main (1)
+# NO-SWIFT-SYMBOLS-NEXT:    Extern
+# NO-SWIFT-SYMBOLS-NEXT:    Type: Section (0xE)
+# NO-SWIFT-SYMBOLS-NEXT:    Section: __text (0x1)
+# NO-SWIFT-SYMBOLS-NEXT:    RefType: UndefinedNonLazy (0x0)
+# NO-SWIFT-SYMBOLS-NEXT:    Flags [ (0x0)
+# NO-SWIFT-SYMBOLS-NEXT:    ]
+# NO-SWIFT-SYMBOLS-NEXT:    Value: 0x100000B70
+# NO-SWIFT-SYMBOLS-NEXT:  }
+# NO-SWIFT-SYMBOLS-NEXT: ]
+
+## Verify that -T does not remove (public) swift symbols when the binary 
+## does not contain __objc_imageinfo in one of the expected segments.
+
+# RUN: yaml2obj -D FLAGS=0x00200085 -D SEGMENT_NAME=__DATA \
+# RUN:  -D SECTION_NAME=__not_objc_imageinfo -D SECTION_SIZE=8 \
+# RUN:  -D SECTION_CONTENT=0000000040070105 %s -o %t4
+# RUN: llvm-strip -x -T %t4
+# RUN: llvm-readobj -symbols %t4 | FileCheck --check-prefix=SWIFT-SYMBOLS %s
+
+# RUN: yaml2obj -D FLAGS=0x00200085 -D SEGMENT_NAME=__NOT_DATA \
+# RUN:  -D SECTION_NAME=__objc_imageinfo -D SECTION_SIZE=8 \
+# RUN:  -D SECTION_CONTENT=0000000040070105 %s -o %t5
+# RUN: llvm-strip -x -T %t5
+# RUN: llvm-readobj -symbols %t5 | FileCheck --check-prefix=SWIFT-SYMBOLS %s
+
+## Verify that -T does not remove (public) swift symbols when swift_version is zero. 
+
+# RUN: yaml2obj -D FLAGS=0x00200085 -D SEGMENT_NAME=__DATA \
+# RUN:  -D SECTION_NAME=__objc_imageinfo -D SECTION_SIZE=8 \
+# RUN:  -D SECTION_CONTENT=0000000000000000 %s -o %t6
+# RUN: llvm-strip -x -T %t6
+# RUN: llvm-readobj -symbols %t6 | FileCheck --check-prefix=SWIFT-SYMBOLS %s
+
+## Verify that -T does not remove (public) swift symbols when the binary 
+## contains invalid (too small) __objc_imageinfo.
+
+# RUN: yaml2obj -D FLAGS=0x00200085 -D SEGMENT_NAME=__DATA \
+# RUN:  -D SECTION_NAME=__objc_imageinfo -D SECTION_SIZE=4 \
+# RUN:  -D SECTION_CONTENT=00000000 %s -o %t7
+# RUN: llvm-strip -x -T %t7
+# RUN: llvm-readobj -symbols %t7 | FileCheck --check-prefix=SWIFT-SYMBOLS %s
+
+## Verify that -T does not remove (public) swift symbols 
+## when the flag MH_DYLDLINK is not set.
+
+# RUN: yaml2obj -D FLAGS=0x00200000 -D SEGMENT_NAME=__DATA \
+# RUN:  -D SECTION_NAME=__objc_imageinfo -D SECTION_SIZE=8 \
+# RUN:  -D SECTION_CONTENT=0000000040070105 %s -o %t8
+# RUN: llvm-strip -x -T %t8
+# RUN: llvm-readobj -symbols %t8 | FileCheck --check-prefix=SWIFT-SYMBOLS %s
+
+# SWIFT-SYMBOLS:      Symbols [
+# SWIFT-SYMBOLS-NEXT:  Symbol {
+# SWIFT-SYMBOLS-NEXT:    Name: _$S1a13PublicSymbol1Sivp (26)
+# SWIFT-SYMBOLS-NEXT:    Extern
+# SWIFT-SYMBOLS-NEXT:    Type: Section (0xE)
+# SWIFT-SYMBOLS-NEXT:    Section: __text (0x1)
+# SWIFT-SYMBOLS-NEXT:    RefType: UndefinedNonLazy (0x0)
+# SWIFT-SYMBOLS-NEXT:    Flags [ (0x0)
+# SWIFT-SYMBOLS-NEXT:    ]
+# SWIFT-SYMBOLS-NEXT:    Value: 0x100001160
+# SWIFT-SYMBOLS-NEXT:  }
+# SWIFT-SYMBOLS-NEXT:  Symbol {
+# SWIFT-SYMBOLS-NEXT:    Name: _$s1a13PublicSymbol2Sivp (1)
+# SWIFT-SYMBOLS-NEXT:    Extern
+# SWIFT-SYMBOLS-NEXT:    Type: Section (0xE)
+# SWIFT-SYMBOLS-NEXT:    Section: __text (0x1)
+# SWIFT-SYMBOLS-NEXT:    RefType: UndefinedNonLazy (0x0)
+# SWIFT-SYMBOLS-NEXT:    Flags [ (0x0)
+# SWIFT-SYMBOLS-NEXT:    ]
+# SWIFT-SYMBOLS-NEXT:    Value: 0x100001168
+# SWIFT-SYMBOLS-NEXT:  }
+# SWIFT-SYMBOLS-NEXT:  Symbol {
+# SWIFT-SYMBOLS-NEXT:    Name: _main (51)
+# SWIFT-SYMBOLS-NEXT:    Extern
+# SWIFT-SYMBOLS-NEXT:    Type: Section (0xE)
+# SWIFT-SYMBOLS-NEXT:    Section: __text (0x1)
+# SWIFT-SYMBOLS-NEXT:    RefType: UndefinedNonLazy (0x0)
+# SWIFT-SYMBOLS-NEXT:    Flags [ (0x0)
+# SWIFT-SYMBOLS-NEXT:    ]
+# SWIFT-SYMBOLS-NEXT:    Value: 0x100000B70
+# SWIFT-SYMBOLS-NEXT:  }
+# SWIFT-SYMBOLS-NEXT: ]
+
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x01000007
+  cpusubtype:      0x80000003
+  filetype:        0x00000002
+  ncmds:           4
+  sizeofcmds:      400
+  flags:           [[FLAGS]]
+  reserved:        0x00000000
+LoadCommands:
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         152
+    segname:         __TEXT
+    vmaddr:          4294967296
+    vmsize:          4096
+    fileoff:         0
+    filesize:        4096
+    maxprot:         5
+    initprot:        5
+    nsects:          1
+    flags:           0
+    Sections:
+      - sectname:        __text
+        segname:         __TEXT
+        addr:            0x0000000100000B70
+        size:            845
+        offset:          0x00000B70
+        align:           4
+        reloff:          0x00000000
+        nreloc:          0
+        flags:           0x80000400
+        reserved1:       0x00000000
+        reserved2:       0x00000000
+        reserved3:       0x00000000
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         152
+    segname:         [[SEGMENT_NAME]]
+    vmaddr:          4294971392
+    vmsize:          4096
+    fileoff:         4096
+    filesize:        4096
+    maxprot:         3
+    initprot:        3
+    nsects:          1
+    flags:           0
+    Sections:
+      - sectname:        [[SECTION_NAME]]
+        segname:         [[SEGMENT_NAME]]
+        addr:            0x0000000100001090
+        size:            [[SECTION_SIZE]]
+        offset:          0x00001090
+        align:           2
+        reloff:          0x00000000
+        nreloc:          0
+        flags:           0x00000000
+        reserved1:       0x00000000
+        reserved2:       0x00000000
+        reserved3:       0x00000000
+        content:         "[[SECTION_CONTENT]]"
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         72
+    segname:         __LINKEDIT
+    vmaddr:          4294975488
+    vmsize:          4096
+    fileoff:         8192
+    filesize:        188
+    maxprot:         1
+    initprot:        1
+    nsects:          0
+    flags:           0
+  - cmd:             LC_SYMTAB
+    cmdsize:         24
+    symoff:          8192
+    nsyms:           5
+    stroff:          8272
+    strsize:         108
+LinkEditData:
+  NameList:
+    - n_strx:          50
+      n_type:          0x1E
+      n_sect:          1
+      n_desc:          0
+      n_value:         4294971760
+    - n_strx:          1
+      n_type:          0x1E
+      n_sect:          1
+      n_desc:          0
+      n_value:         4294971768
+    - n_strx:          74
+      n_type:          0x0F
+      n_sect:          1
+      n_desc:          0
+      n_value:         4294971744
+    - n_strx:          25
+      n_type:          0x0F
+      n_sect:          1
+      n_desc:          0
+      n_value:         4294971752
+    - n_strx:          99
+      n_type:          0x0F
+      n_sect:          1
+      n_desc:          0
+      n_value:         4294970224
+  StringTable:
+    - ''
+    - '_$s1a12LocalSymbol2Sivp'
+    - '_$s1a13PublicSymbol2Sivp'
+    - '_$S1a12LocalSymbol1Sivp'
+    - '_$S1a13PublicSymbol1Sivp'
+    - _main
+    - ''
+    - ''
+    - ''
+...

llvm/tools/llvm-objcopy/COFF/COFFObjcopy.cpp

if (Config.AllowBrokenLinks || !Config.BuildIdLinkDir.empty() ||
       !Config.SplitDWO.empty() || !Config.SymbolsPrefix.empty() ||
       !Config.AllocSectionsPrefix.empty() || !Config.DumpSection.empty() ||
       !Config.KeepSection.empty() || Config.NewSymbolVisibility ||
       !Config.SymbolsToGlobalize.empty() || !Config.SymbolsToKeep.empty() ||
       !Config.SymbolsToLocalize.empty() || !Config.SymbolsToWeaken.empty() ||
       !Config.SymbolsToKeepGlobal.empty() || !Config.SectionsToRename.empty() ||
       !Config.SetSectionAlignment.empty() || Config.ExtractDWO ||
       Config.LocalizeHidden || Config.PreserveDates || Config.StripDWO ||
-      Config.StripNonAlloc || Config.StripSections || Config.Weaken ||
+      Config.StripNonAlloc || Config.StripSections ||
+      Config.StripSwiftSymbols || Config.Weaken ||
       Config.DecompressDebugSections ||
       Config.DiscardMode == DiscardType::Locals ||
       !Config.SymbolsToAdd.empty() || Config.EntryExpr) {
     return createStringError(llvm::errc::invalid_argument,
                              "option not supported by llvm-objcopy for COFF");
   }
 
   return Error::success();

llvm/tools/llvm-objcopy/CopyConfig.h

struct CopyConfig {
   bool OnlyKeepDebug = false;
   bool PreserveDates = false;
   bool StripAll = false;
   bool StripAllGNU = false;
   bool StripDWO = false;
   bool StripDebug = false;
   bool StripNonAlloc = false;
   bool StripSections = false;
+  bool StripSwiftSymbols = false;
   bool StripUnneeded = false;
   bool Weaken = false;
   bool DecompressDebugSections = false;
   DebugCompressionType CompressionType = DebugCompressionType::None;
 
   // parseELFConfig performs ELF-specific command-line parsing. Fills `ELF` on
   // success or returns an Error otherwise.
   Error parseELFConfig() {

llvm/tools/llvm-objcopy/CopyConfig.cpp

Config.DiscardMode =
         InputArgs.hasFlag(STRIP_discard_all, STRIP_discard_locals)
             ? DiscardType::All
             : DiscardType::Locals;
   Config.StripSections = InputArgs.hasArg(STRIP_strip_sections);
   Config.StripUnneeded = InputArgs.hasArg(STRIP_strip_unneeded);
   if (auto Arg = InputArgs.getLastArg(STRIP_strip_all, STRIP_no_strip_all))
     Config.StripAll = Arg->getOption().getID() == STRIP_strip_all;
   Config.StripAllGNU = InputArgs.hasArg(STRIP_strip_all_gnu);
+  Config.StripSwiftSymbols = InputArgs.hasArg(STRIP_strip_swift_symbols);
   Config.OnlyKeepDebug = InputArgs.hasArg(STRIP_only_keep_debug);
   Config.KeepFileSymbols = InputArgs.hasArg(STRIP_keep_file_symbols);
 
   for (auto Arg : InputArgs.filtered(STRIP_keep_section))
     if (Error E = Config.KeepSection.addMatcher(NameOrPattern::create(
             Arg->getValue(), SectionMatchStyle, ErrorCallback)))
       return std::move(E);
 

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

 // handling command line options. It's important to outline certain properties
 // we expect to hold of the command line operations. Any operation that "keeps"
 // should keep regardless of a remove. Additionally any removal should respect
 // any previous removals. Lastly whether or not something is removed shouldn't
 // depend a) on the order the options occur in or b) on some opaque priority
 // system. The only priority is that keeps/copies overrule removes.
 static Error handleArgs(const CopyConfig &Config, Object &Obj,
                         const Reader &Reader, ElfType OutputElfType) {
-
+  if (Config.StripSwiftSymbols)
+    return createStringError(llvm::errc::invalid_argument,
+                             "option not supported by llvm-objcopy for ELF");
   if (!Config.SplitDWO.empty())
     if (Error E =
             splitDWOToFile(Config, Reader, Config.SplitDWO, OutputElfType))
       return E;
 
   if (Config.OutputArch) {
     Obj.Machine = Config.OutputArch.getValue().EMachine;
     Obj.OSABI = Config.OutputArch.getValue().OSABI;

llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp

 
 static void updateAndRemoveSymbols(const CopyConfig &Config, Object &Obj) {
   for (SymbolEntry &Sym : Obj.SymTable) {
     auto I = Config.SymbolsToRename.find(Sym.Name);
     if (I != Config.SymbolsToRename.end())
       Sym.Name = std::string(I->getValue());
   }
 
-  auto RemovePred = [Config](const std::unique_ptr<SymbolEntry> &N) {
+  auto RemovePred = [Config, &Obj](const std::unique_ptr<SymbolEntry> &N) {
     if (N->Referenced)
       return false;
     if (Config.StripAll)
       return true;
     if (Config.DiscardMode == DiscardType::All && !(N->n_type & MachO::N_EXT))
       return true;
+    // This behavior is consistent with cctools' strip.
+    if (Config.StripSwiftSymbols && (Obj.Header.Flags & MachO::MH_DYLDLINK) &&
+        Obj.SwiftVersion && *Obj.SwiftVersion && N->isSwiftSymbol())
+      return true;
     return false;
   };
 
   Obj.SymTable.removeSymbols(RemovePred);
 }
 
 static LoadCommand buildRPathLoadCommand(StringRef Path) {
   LoadCommand LC;

llvm/tools/llvm-objcopy/MachO/MachOReader.h

class MachOReader : public Reader {
   void readRebaseInfo(Object &O) const;
   void readBindInfo(Object &O) const;
   void readWeakBindInfo(Object &O) const;
   void readLazyBindInfo(Object &O) const;
   void readExportInfo(Object &O) const;
   void readDataInCodeData(Object &O) const;
   void readFunctionStartsData(Object &O) const;
   void readIndirectSymbolTable(Object &O) const;
+  void readSwiftVersion(Object &O) const;
 
 public:
   explicit MachOReader(const object::MachOObjectFile &Obj) : MachOObj(Obj) {}
 
   std::unique_ptr<Object> create() const override;
 };
 
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm

llvm/tools/llvm-objcopy/MachO/MachOReader.cpp

for (uint32_t i = 0; i < DySymTab.nindirectsyms; ++i) {
     if ((Index & AbsOrLocalMask) != 0)
       O.IndirectSymTable.Symbols.emplace_back(Index, None);
     else
       O.IndirectSymTable.Symbols.emplace_back(
           Index, O.SymTable.getSymbolByIndex(Index));
   }
 }
 
+void MachOReader::readSwiftVersion(Object &O) const {
+  struct ObjCImageInfo {
+    uint32_t Version;
+    uint32_t Flags;
+  } ImageInfo;
+
+  for (const LoadCommand &LC : O.LoadCommands)
+    for (const std::unique_ptr<Section> &Sec : LC.Sections)
+      if (Sec->Sectname == "__objc_imageinfo" &&
+          (Sec->Segname == "__DATA" || Sec->Segname == "__DATA_CONST" ||
+           Sec->Segname == "__DATA_DIRTY") &&
+          Sec->Content.size() >= sizeof(ObjCImageInfo)) {
+        memcpy(&ImageInfo, Sec->Content.data(), sizeof(ObjCImageInfo));
+        if (MachOObj.isLittleEndian() != sys::IsLittleEndianHost) {
+          sys::swapByteOrder(ImageInfo.Version);
+          sys::swapByteOrder(ImageInfo.Flags);
+        }
+        O.SwiftVersion = (ImageInfo.Flags >> 8) & 0xff;
+        return;
+      }
+}
+
 std::unique_ptr<Object> MachOReader::create() const {
   auto Obj = std::make_unique<Object>();
   readHeader(*Obj);
   readLoadCommands(*Obj);
   readSymbolTable(*Obj);
   setSymbolInRelocationInfo(*Obj);
   readRebaseInfo(*Obj);
   readBindInfo(*Obj);
   readWeakBindInfo(*Obj);
   readLazyBindInfo(*Obj);
   readExportInfo(*Obj);
   readDataInCodeData(*Obj);
   readFunctionStartsData(*Obj);
   readIndirectSymbolTable(*Obj);
+  readSwiftVersion(*Obj);
   return Obj;
 }
 
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm

llvm/tools/llvm-objcopy/MachO/Object.h

struct SymbolEntry {
   bool isExternalSymbol() const { return n_type & MachO::N_EXT; }
 
   bool isLocalSymbol() const { return !isExternalSymbol(); }
 
   bool isUndefinedSymbol() const {
     return (n_type & MachO::N_TYPE) == MachO::N_UNDF;
   }
 
+  bool isSwiftSymbol() const {
+    return StringRef(Name).startswith("_$s") ||
+           StringRef(Name).startswith("_$S");
+  }
+
   Optional<uint32_t> section() const {
     return n_sect == MachO::NO_SECT ? None : Optional<uint32_t>(n_sect);
   }
 };
 
 /// The location of the symbol table inside the binary is described by LC_SYMTAB
 /// load command.
 struct SymbolTable {
struct Object {
   BindInfo Binds;
   WeakBindInfo WeakBinds;
   LazyBindInfo LazyBinds;
   ExportInfo Exports;
   IndirectSymbolTable IndirectSymTable;
   LinkData DataInCode;
   LinkData FunctionStarts;
 
+  Optional<uint32_t> SwiftVersion;
+
   /// The index of LC_SYMTAB load command if present.
   Optional<size_t> SymTabCommandIndex;
   /// The index of LC_DYLD_INFO or LC_DYLD_INFO_ONLY load command if present.
   Optional<size_t> DyLdInfoCommandIndex;
   /// The index LC_DYSYMTAB load comamnd if present.
   Optional<size_t> DySymTabCommandIndex;
   /// The index LC_DATA_IN_CODE load comamnd if present.
   Optional<size_t> DataInCodeCommandIndex;

llvm/tools/llvm-objcopy/StripOpts.td

def no_strip_all : Flag<["--"], "no-strip-all">,
                    HelpText<"Disable --strip-all">;
 
 def d : Flag<["-"], "d">,
         Alias<strip_debug>,
         HelpText<"Alias for --strip-debug">;
 def S : Flag<["-"], "S">,
         Alias<strip_debug>,
         HelpText<"Alias for --strip-debug">;
+
+def strip_swift_symbols : Flag<["-"], "T">,
+                          HelpText<"Remove swift symbols">;