[llvm-objcopy][NFC] Move core implementation of llvm-objcopy into separate library.

[avl]

Summary

This patch moves core implementation of llvm-objcopy into ObjCopy library
(http://lists.llvm.org/pipermail/llvm-dev/2020-September/145075.html).
The functionality for parsing input options is left inside tools/llvm-objcopy.
The interface of ObjCopy library:

ObjCopy/ELF/ELFObjcopy.h

Error executeObjcopyOnIHex(const CopyConfig &Config, MemoryBuffer &In,
                           Buffer &Out);
Error executeObjcopyOnRawBinary(const CopyConfig &Config, MemoryBuffer &In,
                                Buffer &Out);
Error executeObjcopyOnBinary(const CopyConfig &Config,
                             object::ELFObjectFileBase &In, Buffer &Out);

ObjCopy/COFF/COFFObjcopy.h

Error executeObjcopyOnBinary(const CopyConfig &Config,
                             object::COFFObjectFile &In, Buffer &Out);

ObjCopy/MachO/MachOObjcopy.h

Error executeObjcopyOnBinary(const CopyConfig &Config,
                             object::MachOObjectFile &In, Buffer &Out);

ObjCopy/wasm/WasmObjcopy.h

Error executeObjcopyOnBinary(const CopyConfig &Config,
                             object::WasmObjectFile &In, Buffer &Out);

[grimar]

This looks fine to me generally. Have a suggestion though:
in few places there are changes that are related to clang-formatting of original code it seems.
I'd commit them as a separate NFC cleanup (you don't need a review for that) and then rebase this diff.

llvm/unittests/ObjCopy/ObjCopyTest.cpp

39

Perhaps just fail inside?

50

You can avod having this ASSERT_TRUE if you use cast<T>, I think.
Also, you can use T & instead of a pointer, because it is expected that the value is always non-null.

99

Do you need Machine: EM_X86_64? By default it is EM_NONE, so it should work without an explicit value I guess.

[alexander-shaposhnikov]

This change needs some analysis & review, it might take some time (especially because this week we have the LLVM conference).

A few considerations / observations / questions.

1. While this diff moves the implementation of llvm-objcopy into a library it seems like the current interfaces / design are not ideal for a library.

For example, CopyConfig essentially represents parsed command-line options and it is "string-heavy".
Another manifestation of this problem is that the "public" headers (include/llvm/ObjCopy/...) expose a great deal of the internal implementation details etc.

2. Designing a good interface is a nontrivial task. It is important to understand the scope of the problem and potentially dissect it into subparts.

For example if the plan (at least initially) is to reuse the code for reading / writing object files then one of the first steps would be factoring out (and, probably, cleaning up) the model ("class Object") and
exposing the minimal interface for reading / writing . In particular, the internal details of implementation and the associated complexity (e.g. class Reader, class Writer) would live inside the library.

[avl]

addressed comments.

[avl]

@alexshap

This refactoring tries to reuse this part of the functionality:

Error executeObjcopyOnBinary(const CopyConfig &Config, object::ELFObjectFileBase &In, Buffer &Out);

get source file "In", apply transformations described by "Config", write output into the "Out".

i.e. it is not trying to refactor low level of the objcopy code("class Object" and others). It still should be internal thing of ObjCopy library.

I agree that the patch currently makes internal details (class Object, class Reader, class Writer) to be public.
They should be hidden inside library code. I addressed this into the new version of the patch.

[avl]

apply clang-tidy comments.

[jhenderson]

Looks pretty good from my point of view. Some general comments:

1. The functions in the library header need doxygen style comments to explain the interface. Possibly these could be added in a follow-up patch.
2. I think if we are moving files around, now is a good time to run a blanket clang-format on the files. Whether that should be done as part of the patch or a separate follow-on one, I don't know.
3. Probably not this patch, but we should consider our testing strategy for the libray and llvm-objcopy going forward. Some examples for this dicussion: should we move the existing lit tests? Should we port/duplicate some of them as gtest unit tests? How should new library features be tested (gtest or lit)? How should new llvm-strip/objcopy/... options be tested (bearing in mind in theory there should be library testing etc etc)? I think this sort of discussion probably belongs on the mailing list.

I think keeping the surface area of the library down to just the executeObjcopyOn* functions plus the config struct is a pretty good example of a facade style design pattern, and I think makes sense in this context for at least the first version. This assumes that the use-case is a simple in-process read-in/transform/write-out (possibly to a memory buffer for further operations by other tools). We might want to expose other bits of the process and provide other ways of driving the objcopy process in the future (e.g. making it more interactive somehow), but I think that is an extension for later. We do need to nail down the initial use-case(s) though to make sure we don't produce something that isn't useful.

llvm/include/llvm/ObjCopy/Buffer.h

23–24

Maybe as part of a separate patch, it would be worth taking a look at this TODO. It would be great if the Buffer could be removed from the library API and generic LLVM buffers used instead (for example an in-memory buffer or a file buffer, depending on what people want to do).

llvm/include/llvm/ObjCopy/CopyConfig.h

126–127

This comment probably needs updating to better match the new usage - but see out-of-line comment.

llvm/include/llvm/ObjCopy/ObjCopy.h

19–22 ↗

(On Diff #296524)

I think details of the output format shouldn't be described in the comment - theoretically objcopy could even mutate from one object format to another (see e.g. the IHEX stuff).

26–28 ↗

(On Diff #296524)

llvm/lib/ObjCopy/COFF/Object.cpp

1

Whilst you're moving this and the equivalent files for other formats around, could you please rename them to be obvious from the filename which format they are for (same goes for their headers), please? For example, COFF/Object.cpp -> COFF/COFFObject.cpp. The reason for this is that when using the Visual Studio IDE, all the "Object.cpp" files end up listed next to each other in the file browser, and the only way of figuring out which is which is by opening them and seeing.

llvm/tools/llvm-objcopy/wasm/Reader.h

9

[avl]

The functions in the library header need doxygen style comments to explain the interface. Possibly these could be added in a follow-up patch.

Ok.

I think if we are moving files around, now is a good time to run a blanket clang-format on the files. Whether that should be done as part of the patch or a separate follow-on one, I don't know.

Ok, but I think it is better to do in a separate patch.

Probably not this patch, but we should consider our testing strategy for the libray and llvm-objcopy going forward. Some examples for this dicussion: should we move the existing lit tests? Should we port/duplicate some of them as gtest unit tests? How should new library features be tested (gtest or lit)? How should new llvm-strip/objcopy/... options be tested (bearing in mind in theory there should be library testing etc etc)? I think this sort of discussion probably belongs on the mailing list.

OK, I will start that thread soon.

llvm/include/llvm/ObjCopy/Buffer.h

23–24

agreed, but I think it is better to do in separate patch.

llvm/include/llvm/ObjCopy/CopyConfig.h

126–127

ok.

llvm/include/llvm/ObjCopy/ObjCopy.h

19–22 ↗

(On Diff #296524)

ok.

llvm/lib/ObjCopy/COFF/Object.cpp

1

ok.

[alexander-shaposhnikov]

I have some general comments / concerns (in addition to the inline comment).
The interface of the library is important and once it's committed and people start using the library in multiple places it might be harder to make changes / fix issues
(unfortunately this has already happened in LLVM a few times in the past) .

1. CopyConfig as a part of the interface in its current form seems to be not "in a perfect shape". E.g. some ELF-specific options are inside the struct ELF, some others are just regular fields.

Some fields make sense in the context of the tool but they don't (at least at quick glance) in the context of a library function.

2. class Buffer - I agree with @jhenderson's comment. Just want to note that in general there are multiple ways how to address it, but this requires some thinking.

1, 2 - perhaps, this can be refactored / reorganized / better documented, but it would be great to have it done before the introduction of the library.

I'd like to point out that not all the operations supported by llvm-objcopy can be performed in-memory (e.g. splitting dwo will create a new file). If a library function has such side effects (creates new objects on the local file system) it would be good to have it documented.

cc: @mtrent , @dblaikie, @echristo

llvm/lib/ObjCopy/COFF/Object.cpp

1

@jhenderson, I'm sorry to disagree, but renaming files this way doesn't seem to be a good idea and the provided justification doesn't appear to be sufficient.
Since this file contains the implementation of what's declared in Object.h I would strongly prefer to have it named Object.cpp given it is already located in the corresponding folder. Visual Studio IDE might have some peculiarities but having consistent naming is important, adding such prefixes doesn't seem to be a good approach.

[avl]

1, 2 - perhaps, this can be refactored / reorganized / better documented, but it would be great to have it done before the introduction of the library.

if possible - I propose to do these points as next patches after this(to make some progress on it). If not - I would work on preliminary patches.

llvm/lib/ObjCopy/COFF/Object.cpp

1

Would it be OK, If both of the files would be renamed Object.h -> COFFObject.h and Object.cpp->COFFObject.cpp ?

[avl]

llvm/include/llvm/ObjCopy/Buffer.h

23–24

@alexshap @jhenderson

Speaking of class Buffer refactoring. I do not think that this comment is fully valid:

// The class Buffer abstracts out the common interface of FileOutputBuffer and
// WritableMemoryBuffer so that the hierarchy of Writers depends on this
// abstract interface and doesn't depend on a particular implementation.
// TODO: refactor the buffer classes in LLVM to enable us to use them here
// directly.

It suggests to create some common interface for FileOutputBuffer and WritableMemoryBuffer.
Which is assumed to look similar to this:

class Buffer {
  StringRef Name;

public:
  virtual ~Buffer();
  virtual Error allocate(size_t Size) = 0;
  virtual uint8_t *getBufferStart() = 0;
  virtual Error commit() = 0;

  explicit Buffer(StringRef Name) : Name(Name) {}
  StringRef getName() const { return Name; }
};

There exists a problem with methods commit() and allocate(). commit() is a redundant for WritableMemoryBuffer.
adding it to WritableMemoryBuffer would require to patch all current usages of WritableMemoryBuffer.
So it looks incorrectly to use it for common parent interface of FileOutputBuffer and WritableMemoryBuffer.
allocate() suggests another way of buffer creation. Currently, buffers are created by static creation methods:

static std::unique_ptr<WritableMemoryBuffer>
getNewMemBuffer(size_t Size, const Twine &BufferName = "");

static Expected<std::unique_ptr<FileOutputBuffer>>
create(StringRef FilePath, size_t Size, unsigned Flags = 0);

Adding "virtual Error allocate(size_t Size)" would lead to creation empty buffer by static creation method
and then call to allocate(). This does not seem a good addition to the already existed FileOutputBuffer and WritableMemoryBuffer. FileOutputBuffer and WritableMemoryBuffer assume another use cases than Buffer.

Actually, what is neccessary by objcopy is method:

uint8_t *createBuffer ( size_t Size );

All other functionality is redundant and could be removed from objcopy.

what do you think about following design?

LazyBuffer {
  StringRef Name;

  virtual StringRef getName() const { return Name; }
  virtual uint8_t *createBuffer ( size_t Size ) = 0;
};

MemoryLazyBuffer : public LazyBuffer {
  virtual uint8_t *createBuffer ( size_t Size ) {
    Buffer = WritableMemoryBuffer::getNewMemBuffer(Size, Name);
    return Buffer->getBufferStart();
  }

  std::unique_ptr<WritableMemoryBuffer> Buffer;
};

FileLazyBuffer : public LazyBuffer {
  virtual uint8_t *createBuffer ( size_t Size ) {
    Buffer = FileOutputBuffer::create(Name, Size);
    return Buffer->getBufferStart();
  }

  std::unique_ptr<FileOutputBuffer> Buffer;
};

Usage:

static Error executeObjcopyOnBinary(CopyConfig &Config, object::Binary &In,
                                    LazyBuffer &Out);


MemoryLazyBuffer MB("name");
executeObjcopyOnBinary(Config, Input, MB);

FileLazyBuffer MB("name");
executeObjcopyOnBinary(Config, Input, MB);
if (MB.Buffer)
  MB.Buffer->commit();

Another alternative is that library always writes to general MemoryBuffer :

static Expected<std::unique_ptr<MemoryBuffer>> executeObjcopyOnBinary(CopyConfig &Config, object::Binary &In);

and later this MemoryBuffer would be written into the file by llvm-objcopy.cpp.

What do you think?

[echristo]

I like the direction this is going, I'll take more of a deep look soon, but wanted to ask: "should this be in Object rather than a separate library?" When I'd originally asked for this to be split into its own library I'd thought that it would get added into libobject.

Thoughts?

-eric

[jhenderson]

In D88827#2322005, @echristo wrote:

I like the direction this is going, I'll take more of a deep look soon, but wanted to ask: "should this be in Object rather than a separate library?" When I'd originally asked for this to be split into its own library I'd thought that it would get added into libobject.

Thoughts?

-eric

One of the ideals would be to have writable versions of the various ObjectFile classes defined by the Object library. I know this was worked on last year as part of GSOC by @abrachet, but it didn't really end up getting to a usable point. I kind of like the separation of concerns here - the proposed Objcopy library would handle manipulation of object files, whilst the Object library is primarily for parsing and inspecting them. The former builds on the latter, but the latter doesn't need to care about the former. Thus a user who wrote an object dumping tool wouldn't need the Objcopy library. However, I don't have a strong opinion on this, so if a good design could be presented to resolve that, I'd be happy.

In D88827#2319015, @alexshap wrote:

I have some general comments / concerns (in addition to the inline comment).
The interface of the library is important and once it's committed and people start using the library in multiple places it might be harder to make changes / fix issues
(unfortunately this has already happened in LLVM a few times in the past) .

Could we maybe put a big note at the top of the library header saying that the API is still work-in-progress and shouldn't be used, until we've got to the final point? I feel like moving the code first is a good step and then we can iterate on the design once it's in place (I agree that CopyConfig probably needs more work).

llvm/include/llvm/ObjCopy/Buffer.h

23–24

I'm not sure I've looked at the intricacies of the different buffer classes to know what the right approach is. However, if modifying the existing buffers seems like it won't work/will be too invasive, this seems like a fair approach (it's the "Adapter" design pattern in action, I believe).

I'm not sure I'd call it LazyBuffer, although I don't have a specific better name (maybe just ObjcopyBuffer, depending on how generic we want it to be). Also, unless the objcopy code actually needs the name for anything (aside from error messages, I'm not sure what that would be), I'd not include that in the interface.

[avl]

In D88827#2322005, @echristo wrote:

I like the direction this is going, I'll take more of a deep look soon, but wanted to ask: "should this be in Object rather than a separate library?" When I'd originally asked for this to be split into its own library I'd thought that it would get added into libobject.

Thoughts?

I think, for the start, it is probably better to make it as a separate library. It could be refactored later and some part could probably be moved into Object library(Object.h/.cpp, Reader.h/.cpp, /Writer.h/.cpp).
If the ObjCopy library would be used in many other places in the end - then it would make sense to put it into the Object library, otherwise it would be better to live it as separate library.
Though, I am not against moving this ObjCopy library into the Object library, if there is opinion that it would be better.

[avl]

llvm/lib/ObjCopy/COFF/Object.cpp

1

@jhenderson, I'm sorry to disagree, but renaming files this way doesn't seem to be a good idea and the provided justification doesn't appear to be sufficient.
Since this file contains the implementation of what's declared in Object.h I would strongly prefer to have it named Object.cpp given it is already located in the corresponding folder. Visual Studio IDE might have some peculiarities but having consistent naming is important, adding such prefixes doesn't seem to be a good approach.

@alexshap Could you explain this renaming thing, please? i.e. if both header file COFF/Object.h and src file COFF/Object.cpp would be renamed(COFF/COFFObject.h, COFF/COFFObject.cpp), would it be OK?

[avl]

addressed comments: renamed files, added doxygen comments into library headers(except CopyConfig.h).

I propose to go ahead with this patch and continue with follow-up patches:

1. Remove Buffer.h/Buffer.cpp.
2. cleanup CopyConfig.h.
3. remove handling of SplitDWO from the library.

[echristo]

I really do think this needs to be under object. It can be separate in there as lib/Object/Copy if you want, but I don't think it should be a parallel directory. Hopefully the updates for the move aren't overly onerous here.

-eric

[sbc100]

In D88827#2344900, @echristo wrote:

I really do think this needs to be under object. It can be separate in there as lib/Object/Copy if you want, but I don't think it should be a parallel directory. Hopefully the updates for the move aren't overly onerous here.

-eric

Won't that mean that any tool that just wants to read in object files (e.g. lld) will need to build this extra code for no reason? Not a huge deal I guess (especially for lld which due to LTO pulls in all of llvm!) but I imagine there are many tools that just want to read (and not modify) objects.

[alexander-shaposhnikov]

My 0.02$: (perhaps, this should have been mentioned earlier) the current class CopyConfig contains e.g. file names (again, imo it is good enough for a tool, but not good enough for a library) and this means that if somebody wants to add a section to an object file he won't be able to accomplish this task using the current interface without creating extra files. It kind of defeats the idea. To solve this problem proper abstractions should be introduced / the code needs to be refactored. Personally I would strongly prefer to see the following iterative approach here: refactor the current code in llvm-objcopy step by step until it's ready to be moved into a library with a clean and easy-to-use interface. Maybe I'm missing something, but doing refactoring post factum seems to be a less controllable process and might get us to the state where the code has been move out of the tool, the interface has been modified to accomplish a very specific task and the rest (burden) will stay there for years creating more issues than benefits, moreover, it would introduce some risks.
Regarding where to place these functions - into libObject or create a separate library - libObject already contains several write* functions, (e.g. for archives), so indeed, putting this group of functions (e.g. one can use a bit less verbose name - copy(...)) into libObject seems to be quite natural.

llvm/lib/ObjCopy/COFF/Object.cpp

1

I'm very sorry, but i still think that the old names were good, adding these prefixes is unnecessary and makes things less intuitive (e.g. class Object is described in Object.h).

[jhenderson]

In D88827#2344916, @sbc100 wrote:

In D88827#2344900, @echristo wrote:

I really do think this needs to be under object. It can be separate in there as lib/Object/Copy if you want, but I don't think it should be a parallel directory. Hopefully the updates for the move aren't overly onerous here.

-eric

Won't that mean that any tool that just wants to read in object files (e.g. lld) will need to build this extra code for no reason? Not a huge deal I guess (especially for lld which due to LTO pulls in all of llvm!) but I imagine there are many tools that just want to read (and not modify) objects.

That will depend on the archiver/linker behaviour, I expect, but at least for standard ld.lld behaviour (I can't speak for others), only the objects that actually are needed are pulled from the archive. Therefore, for example, if LLD didn't reference anything in the objcopy codebase, it wouldn't actually use those archive members, and therefore it wouldn't be added to the tools code. Even if it did, linker options like --gc-sections or equivalent likely would cause the unused code to be removed at link time, if the tool is built appropriately.

@alexshap's point about other files in the Object library allowing writing is persuasive to me. I think an Objcopy sub-directory would make sense though, to avoid the confusion of having both an ObjectFile and Object class/header file etc in Object. This is assuming of course we don't want to jump into refactoring the two classes so that they become one. I suspect that way might prove tricky to get right.

I'm somewhat ambivalent as to whether we move then refactor or refactor then move. If people feel that the latter approach is the right one, then so be it.

Regarding the naming of Object.h, that's fine, leave it as-is. I suspect with some small CMake changes, it should be possible to get the files to appear in a corresponding "solution folder" within the VS IDE, which would help disambiguate things.

[sbc100]

In D88827#2346567, @jhenderson wrote:

In D88827#2344916, @sbc100 wrote:

In D88827#2344900, @echristo wrote:

I really do think this needs to be under object. It can be separate in there as lib/Object/Copy if you want, but I don't think it should be a parallel directory. Hopefully the updates for the move aren't overly onerous here.

-eric

Won't that mean that any tool that just wants to read in object files (e.g. lld) will need to build this extra code for no reason? Not a huge deal I guess (especially for lld which due to LTO pulls in all of llvm!) but I imagine there are many tools that just want to read (and not modify) objects.

That will depend on the archiver/linker behaviour, I expect, but at least for standard ld.lld behaviour (I can't speak for others), only the objects that actually are needed are pulled from the archive. Therefore, for example, if LLD didn't reference anything in the objcopy codebase, it wouldn't actually use those archive members, and therefore it wouldn't be added to the tools code. Even if it did, linker options like --gc-sections or equivalent likely would cause the unused code to be removed at link time, if the tool is built appropriately.

You are right, the size of resulting tools won't be increased. I was referring to the fact that such a tool would transitively depend on more sources and would take longer to compile, and would become dirty more often on average after a git pull. For example, its good that one can build llvm-objdump without compiling the whole of llvm.

@alexshap's point about other files in the Object library allowing writing is persuasive to me. I think an Objcopy sub-directory would make sense though, to avoid the confusion of having both an ObjectFile and Object class/header file etc in Object. This is assuming of course we don't want to jump into refactoring the two classes so that they become one. I suspect that way might prove tricky to get right.

I'm somewhat ambivalent as to whether we move then refactor or refactor then move. If people feel that the latter approach is the right one, then so be it.

Regarding the naming of Object.h, that's fine, leave it as-is. I suspect with some small CMake changes, it should be possible to get the files to appear in a corresponding "solution folder" within the VS IDE, which would help disambiguate things.

[avl]

To summarize comments - I am going to:

1. create preliminary patch for Buffers.
2. create preliminary patch for CopyConfig.
3. move handling of SplitDWO into llvm-objcopy.cpp.
4. move this functionality into Object library.
5. do not rename files.

[avl]

@alexshap @jhenderson @echristo

Folks, Before creating a patch I would like to consult what would be the best option to refactor the Buffer class.
One of the alternatives is described here https://reviews.llvm.org/D88827#2321871 ("Adapter" approach).

I think the better solution would be to use raw_ostream instead of buffers:

current:

Error executeObjcopyOnBinary(const CopyConfig &Config,
                             object::ELFObjectFileBase &In, Buffer &Out);

new:

Error executeObjcopyOnBinary(const CopyConfig &Config,
                             object::ELFObjectFileBase &In, raw_ostream &Out);

Generally, using streams could allow us to reduce memory usages. No need to load all data into the memory - the data could be streamed through a smaller buffer. Opposite, the current WritableMemoryBuffer, used in llvm-objcopy, would allocate whole data into the memory. Thus replacing WritableMemoryBuffer with raw_ostream
would allow minimizing memory requirements.

FileOutputBuffer(used by llvm-objcopy) has an advantage over raw_fd_ostream(which might be used if we would like
to store data into the file). FileOutputBuffer::createOnDiskBuffer() allows to use memory mapped file. The similar functionality
could be implemented for raw_fd_ostream. It is possible to add preallocate() method into raw_ostream.

class raw_ostream {

void preallocate(uint64_t size);

}

That method, implemented for raw_fd_ostream, could create a memory-mapped file. The streamed data would be written
into that memory file then. Thus we would be able to use memory-mapped files with raw_fd_ostream.

So, it seems we could use raw_ostream instead of Buffer without losing functionality.
It seems to me that raw_ostream is a good abstraction here and it would be good to use it for llvm-objcopy.

So what is your opinion, would it be OK to use raw_ostream?
Or should we use "Adapter" approach, from https://reviews.llvm.org/D88827#2321871 ?

[jhenderson]

Using a stream approach sounds reasonable. I don't really know what the benefits are of using a memory mapped file versus other options (I vaguely recall from some older work that they improve performance, but am not sure if that is still the case or not). The one concern I'd have with a stream for writing output is if we ever need to jump back and forth within the object for some reason. Without looking at the existing objcopy code, I don't know if there are any instances where this happens though.

By the way, I think the preallocate method might be better termed reserve as it sounds like it solves a similar intent as std::vector::reserve to me.

[avl]

Using a stream approach sounds reasonable. I don't really know what the benefits are of using a memory mapped file versus other options (I vaguely recall from some older work that they improve performance, but am not sure if that is still the case or not). The one concern I'd have with a stream for writing output is if we ever need to jump back and forth within the object for some reason. Without looking at the existing objcopy code, I don't know if there are any instances where this happens though.

If there is such necessity then instead of raw_ostream there could be used raw_pwrite_stream, which allows such seek&update functionality.

During this refactoring effort, I am not going to rewrite the existing objcopy writing code.
So I plan to do things this way:

Error executeObjcopyOnBinary(Config,In, raw_ostream &Out) {
  // TODO: refactor "writing" code to output into "raw_ostream &Out"
  // TOFO: directly, without MemBuffer in the middle.
  MemBuffer.allocate(Size)
  // existing writing code.
  MemBuffer.commit();
  Out.write(MemBuffer.getBufferStart(), MemBuffer.getBufferSize());
}

I propose to use raw_ostream now and replace it with raw_pwrite_stream later, if it would be necessary.
If it would be hard to use raw_ostream during such postponed rewriting then we could change raw_ostream into raw_pwrite_stream at that moment.

If it is already clear that we need to use raw_pwrite_stream then I will use it within my patch.

By the way, I think the preallocate method might be better termed reserve as it sounds like it solves a similar intent as std::vector::reserve to me.

Ok

[avl]

addressed comments(usages of Buffers are replaced with streams, CopyConfig is refactored,
handling of SplitDWO is moved into llvm-objcopy.cpp, implementation of
llvm-objcopy is moved into the Object library).

[avl]

@jhenderson @alexander-shaposhnikov

Would you mind to take a look at this review, please? It implemented all requests which were done previously https://reviews.llvm.org/D88827#2349119.

[jhenderson]

In D88827#3003632, @avl wrote:

@jhenderson @alexander-shaposhnikov

Would you mind to take a look at this review, please? It implemented all requests which were done previously https://reviews.llvm.org/D88827#2349119.

Sorry I haven't got to this - there are seeral higher-priority items that I need to be focusing on. It's still on my radar though.

[avl]

ping,

[jhenderson]

In D88827#3118304, @avl wrote:

ping,

Apologies for not getting to this - a number of other reviews have used up what time I've had to do reviews.

This patch is looking very close to being ready, but not quite there yet.

llvm/include/llvm/Object/ObjCopy/ConfigManager.h

24–25 ↗

(On Diff #372901)

This should be a doc comment if it is needed. I don't think the comment is particularly useful in its current form. The functionality is fairly obvious from its interface.

33 ↗

(On Diff #372901)

The functionality in this and similar functions for wasm and Mach-O should be moved to a .cpp file. It's not unlikely that it will change over time, so we don't want to force rebuilds due to it being unnecessarily in the header.

llvm/include/llvm/Object/ObjCopy/ObjCopy.h

12–18 ↗

(On Diff #372901)

I think you can forward declare most of these classes, and include the headerse in the .cpp rather than having this header include them all.

I think about the only ones you likely need are Error.h, and vector.

llvm/lib/ObjCopy/COFF/Object.cpp

1

Actually, I think the way to solve this is to use some CMake functionality to stick specific files in IDE folders, so that they don't end up clashing: in the Visual Studio IDE, for example, I see 4 Object.h files next to each other in the Header Files group, but there's no indication which is which. Instead, we could create COFF, ELF etc sub-groups for the headers and source files. I believe https://cmake.org/cmake/help/latest/command/source_group.html is the relevant piece of CMake.

llvm/lib/Object/CMakeLists.txt

32–47 ↗

(On Diff #372901)

I thought ObjCopy was going to be its own distinct library, not a part of the Object library? It would make more sense to me for it to be separate, as it's a fairly distinct piece of functionality.

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

26–28

I've no issue with adding comments like these, but could you spin them off into separate patches, please, as they are independently useful, and this will help reduce the size of this patch.

Same applies for other new doc comments.

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

171

As you're renaming files, it's probably worth doing a complete clang-format of the whole file at the same time. Applies to all files with clang-format issues in otherwise untouched code.

llvm/unittests/Object/ObjCopyTest.cpp

9 ↗

(On Diff #372901)

Here and in other new files, if you haven't already, please make sure the include set is minimal for the code in the new file.

35 ↗

(On Diff #372901)

As this function is called from several places, you may want to add some tracing to it: https://github.com/google/googletest/blob/main/docs/advanced.md#adding-traces-to-assertions

If you don't, and the test fails, you may not be able to easily tell which test is causing the failure.

39 ↗

(On Diff #372901)

I believe YAML is an acronym, so should be all-caps in comments.

49–50 ↗

(On Diff #372901)

I was under the impression that temporary files are supposed to be deleted automatically (I might be wrong), so you wouldn't need the FileRemover?

More generally, the need to create a concrete file in the unit test makes me wonder whether we could enhance the objcopy API to take an object of some kind that could represent data in memory (as an alternative to on-disk). I believe such an object hierarchy already exists within LLVM, although the name escapes me right now.

[avl]

llvm/lib/Object/CMakeLists.txt

32–47 ↗

(On Diff #372901)

It was directly requested to implement it as part of Object library during the previous review iteration:

https://reviews.llvm.org/D88827#2344900

[jhenderson]

llvm/lib/Object/CMakeLists.txt

32–47 ↗

(On Diff #372901)

Right, there was some opposition from @sbc100 though. I think we need to hear their thoughts on that.

If we do end up putting it in libObject, should the namespace be changed? Not sure either way.

[avl]

llvm/lib/Object/CMakeLists.txt

32–47 ↗

(On Diff #372901)

Do you mean:

objcopy->object?
objcopy->objcopybase?

I think it is fine to have objcopy(like in this patch). Changing objcopy->object creates some names clashes. Thus we need to have other namespace than object for the objcopy part. Having objcopy inside Object library and inside llvm-objcopy tool seems to be no problem.

[jhenderson]

llvm/lib/Object/CMakeLists.txt

32–47 ↗

(On Diff #372901)

I was referring to the first of those (using object instead of objcopy). If using the same namespace would be a problem, I don't see any issue with leaving as-is.

[avl]

llvm/unittests/Object/ObjCopyTest.cpp

49–50 ↗

(On Diff #372901)

I was under the impression that temporary files are supposed to be deleted automatically (I might be wrong), so you wouldn't need the FileRemover?

I do not see the place where automatic removing is implemented. Also, there are examples of FileRemover usages in other unit tests:

https://github.com/llvm/llvm-project/blob/7b6790850968031fe1c098ed6dcc196ddc547ea5/llvm/unittests/Support/MemoryBufferTest.cpp#L105

More generally, the need to create a concrete file in the unit test makes me wonder whether we could enhance the objcopy API to take an object of some kind that could represent data in memory (as an alternative to on-disk). I believe such an object hierarchy already exists within LLVM, although the name escapes me right now.

I think it would be a good enhancement. I propose to do it in a separate patch though.

[jhenderson]

llvm/unittests/Object/ObjCopyTest.cpp

49–50 ↗

(On Diff #372901)

I do not see the place where automatic removing is implemented

For Windows, see references to the OF_Delete flag in Path.inc. For Linux, the behaviour is configured higher up the stack. See RemoveFileOnSignal referenced by TempFile::create.

At the location you cited, FileRemover is needed, because we're not working with a TempFile there.

I think it would be a good enhancement. I propose to do it in a separate patch though.

Agreed, but I think it would be useful to see it before this patch gets committed, to avoid a bad interface landing.

[avl]

llvm/unittests/Object/ObjCopyTest.cpp

49–50 ↗

(On Diff #372901)

Ah, the suggestion is to use sys::fs::TempFile::create instead of sys::fs::createTemporaryFile(). I see, thanks.

[jhenderson]

llvm/unittests/Object/ObjCopyTest.cpp

49–50 ↗

(On Diff #372901)

I didn't realise the two did different things, but it would make sense to do as you are saying, yes.

[avl]

llvm/unittests/Object/ObjCopyTest.cpp

49–50 ↗

(On Diff #372901)

I think we could not use sys::fs::TempFile::create() for reusing automatic removing. Please look at the use case:

Expected<sys::fs::TempFile> Temp =
  sys::fs::TempFile::create("a.temp-unittest-%%%%%%", Mode);

Config.Common.OutputFilename = Temp.TmpName;

// Call executeObjcopyOnBinary()
std::error_code EC;
raw_fd_ostream OutStream(Temp->FD, false);
Error Err = objcopy::executeObjcopyOnBinary(Config, *Obj.get(), OutStream);
OutStream.flush();

// if we call Temp.keep() here then the autoremoving functionality would be lost.
// if we call Temp.discard() then the file would be removed.

// if we not call Temp.keep() or Temp.discard()
// then we would try to open the temporarily file second time
// (inside ObjectFile::createObjectFile) which is undesirable.

// Load and check copied file.
Expected<OwningBinary<ObjectFile>> Result =
    ObjectFile::createObjectFile(Config.Common.OutputFilename);

[jhenderson]

llvm/unittests/Object/ObjCopyTest.cpp

49–50 ↗

(On Diff #372901)

Fair enough. Stick with this for now, although that maybe suggests we should change the executeObjcopyOnBinary interface sooner rather than later, so that it can take an in-memory buffer, instead of an on-disk file.

[avl]

addressed comments:

1. deleted doc comments(will add them with the follow-up patch).
2. moved methods implementation into .cpp file.
3. optimized headers.
4. did not modify cmake to create IDE folders(will do it with the follow-up patch).
5. added traces in the test assertions.
6. rewrote test to use inmemory output file.

[jhenderson]

A couple of nits remaining, otherwise basically looks good. I'd still like:

1. @sbc100's response to the library division. I have a very slight preference for separate, but don't care enough about it to push for it, if others are opposed. Adding them as a reviewer in case it gets their attention.
2. I'd like to see patches based on this patch for the other issues that your deferring (particularly the IDE one).

Once those are dealt with, I'll give this an LGTM.

llvm/include/llvm/Object/ObjCopy/ObjCopy.h

12 ↗

(On Diff #388956)

I think you can forward declare Binary, Archive and NewArchiveMember, right, so that you don't need this header?

llvm/tools/llvm-objcopy/ObjcopyOptions.cpp

627 ↗

(On Diff #388956)

More clang-formatting that can be done as you rename the file.

[jhenderson]

Oh, ideally I'd also get a second reviewer to give this a once-over, in case I've missed anything too.

[avl]

@sbc100's response to the library division. I have a very slight preference for separate, but don't care enough about it to push for it, if others are opposed. Adding them as a reviewer in case it gets their attention.

My own preference is also to make ObjCopy to be separate library, though I also do not want to push it if others want opposite. I read this message https://reviews.llvm.org/D88827#2344900 and this https://reviews.llvm.org/D88827#2346399 and this https://reviews.llvm.org/D88827#2346567 responses as a consensus on making it to be part of Object library. If we do not have a consensus, I am open to discuss it further.

llvm/include/llvm/Object/ObjCopy/ObjCopy.h

12 ↗

(On Diff #388956)

std::vector<NewArchiveMember> wants to know sizeof (NewArchiveMember).
So, we could not have forward declaration here. i.e. we still need ArchiveWriter.h header.

[avl]

addressed clang-format issue.

[jhenderson]

In D88827#3148315, @avl wrote:

@sbc100's response to the library division. I have a very slight preference for separate, but don't care enough about it to push for it, if others are opposed. Adding them as a reviewer in case it gets their attention.

My own preference is also to make ObjCopy to be separate library, though I also do not want to push it if others want opposite. I read this message https://reviews.llvm.org/D88827#2344900 and this https://reviews.llvm.org/D88827#2346399 and this https://reviews.llvm.org/D88827#2346567 responses as a consensus on making it to be part of Object library. If we do not have a consensus, I am open to discuss it further.

I changed my mind a bit since my early comment you linked, having gone cold on the review and now having come back to it. Here's my thinking: the object manipulation performed by the objcopy code is based on an internal Object class that has nothing to do with the object classes within the libObject library. By putting the two inside the same library, we risk confusion ("which type of Object do I need for this functionality?"). There are fundamental differences which would make reusing the libObject classes for the objcopy code less than ideal - it may not even be possible without a lot of work.

Regarding the Archive writing code being in libObject: it's my tentative opinion that archive functionality shouldn't be in libObject at all: an archive isn't really an object itself: it's a group of objects. The functionality for archives is completely unrelated to the functionality for other file types supported by the object library. As such, I think it should actually be moved into a separate libArchive library or similar.

Finally, there's @sbc100's point earlier: whilst it's true that by having the code in the same library doesn't force tools to link in that code, it does require distributions to include the llvm-objcopy library code anytime it needs the libObject code, because it's part of that library, even though it may not need it. I've seen several instances internally where we generate packages of LLVM libraries, without including all of them, so including the llvm-objcopy code in the libObject library would bloat the size of the package.

@echristo, @alexander-shaposhnikov, do either of you have any further thoughts? What do you think of my points above? I'm not strongly opposed to it going in libObject, but just think it makes a little more sense to be separate. Also, would either of you be able to give this a review more generally?

llvm/include/llvm/Object/ObjCopy/ObjCopy.h

12 ↗

(On Diff #388956)

You learn something new everyday. I thought the type used in std::vector could also be forward declared if it was just used in the return signature. Guess I was wrong.

[avl]

@echristo, @alexander-shaposhnikov Do you have objections to doing Objcopy to be a separate library instead of putting it into the existing Object library?

I agree with concerns raised by @jhenderson (https://reviews.llvm.org/D88827#3148452) and @sbc100 (https://reviews.llvm.org/D88827#2348439): Doing Objcopy as a separate library would allow to reduce code inter-dependency and reduce size of resulting code/package(when objcopy functionality is not neccessary).

[avl]

@echristo, @alexander-shaposhnikov ping...

[avl]

@echristo, @alexander-shaposhnikov Would you mind taking part in this review, please? Your opinion might help to make a progress with it. During previous iterations, you preferred the variant when ObjCopy code would be moved into the Object library. There are arguments to make ObjCopy be a separate library. Do you have objections to that?

[avl]

@echristo, @alexander-shaposhnikov ping...

[avl]

@jhenderson James, what do you think would be right to do if we do not have a response from others?

Both solutions(make it be a separate library or make it be part of Object) are OK for me.
It would be good if we might have a progress with this patch. Probably we can make a decision and continue with it?

[jhenderson]

@avl, I'd suggest emailing them directly to prod them, as they aren't responding. If they don't respond after that, I think you should adopt what active reviewers (i.e. me!) are suggesting.

On the note of activity, I will be off for at least 2 weeks, starting tomorrow, so may not be around to continue reviewing in that time!

[avl]

In D88827#3240313, @jhenderson wrote:

@avl, I'd suggest emailing them directly to prod them, as they aren't responding. If they don't respond after that, I think you should adopt what active reviewers (i.e. me!) are suggesting.

On the note of activity, I will be off for at least 2 weeks, starting tomorrow, so may not be around to continue reviewing in that time!

I see, thanks! will follow that advice.

[echristo]

In D88827#3148452, @jhenderson wrote:

In D88827#3148315, @avl wrote:

@sbc100's response to the library division. I have a very slight preference for separate, but don't care enough about it to push for it, if others are opposed. Adding them as a reviewer in case it gets their attention.

My own preference is also to make ObjCopy to be separate library, though I also do not want to push it if others want opposite. I read this message https://reviews.llvm.org/D88827#2344900 and this https://reviews.llvm.org/D88827#2346399 and this https://reviews.llvm.org/D88827#2346567 responses as a consensus on making it to be part of Object library. If we do not have a consensus, I am open to discuss it further.

I changed my mind a bit since my early comment you linked, having gone cold on the review and now having come back to it. Here's my thinking: the object manipulation performed by the objcopy code is based on an internal Object class that has nothing to do with the object classes within the libObject library. By putting the two inside the same library, we risk confusion ("which type of Object do I need for this functionality?"). There are fundamental differences which would make reusing the libObject classes for the objcopy code less than ideal - it may not even be possible without a lot of work.

The original idea is that these classes -could- be used for this. If that's not true we should go back and fix that rather than have a separate set. That said, we do have a separate set which complicates matters. I think the question I'd ask is "how can we get from here to there?". No straightforward answer here for sure.

Regarding the Archive writing code being in libObject: it's my tentative opinion that archive functionality shouldn't be in libObject at all: an archive isn't really an object itself: it's a group of objects. The functionality for archives is completely unrelated to the functionality for other file types supported by the object library. As such, I think it should actually be moved into a separate libArchive library or similar.

From my perspective: ideally an archive should just be a specific type of object that contains other objects (similar to perhaps a shared library if we did that). Reading it just means cracking it, sharing the archive format information, and then iterating through each object accordingly. Writing would be the reverse. Any thoughts on where we're on a different page? :)

Finally, there's @sbc100's point earlier: whilst it's true that by having the code in the same library doesn't force tools to link in that code, it does require distributions to include the llvm-objcopy library code anytime it needs the libObject code, because it's part of that library, even though it may not need it. I've seen several instances internally where we generate packages of LLVM libraries, without including all of them, so including the llvm-objcopy code in the libObject library would bloat the size of the package.

Are you thinking about things just shipping .a or .o files or something else? Most platforms that I can think of can remove unneeded object code in a binary link so I'm curious about the concern here.

@echristo, @alexander-shaposhnikov, do either of you have any further thoughts? What do you think of my points above? I'm not strongly opposed to it going in libObject, but just think it makes a little more sense to be separate. Also, would either of you be able to give this a review more generally?

I'll do my best :)

-eric

[MaskRay]

llvm-objcopy has its own data models for some binary format specific things, so placing it under the llvm::object namespace may lead to some conflicts. Placing it into a sub-namespace does not avoid the conflict.
If it keeps using the llvm::objcopy namespace, then placing it under lib/Object may cause some confusion. A separate directory looks good to me.

---

An archive is a quite different container format. So I get the point that there is a question about whether it fits into lib/Object.
On the other hand, an archive is sometimes an input file type and used nearly indistinguishable with other file typed in syntax (ld.lld, llvm-readelf, ...). Having it in lib/Object seems fine to me...

---

By placing some less associated code in the same library, there is a risk that dependencies tend to become mixed.
Layered component design has some advantages but in practice it is easy to run into a situation that for all of {a,b,c}.cpp to include all of {a,b,c}.h...
Different lib/* directories tend to force developers to think harder on the layering.

[jhenderson]

In D88827#3242326, @echristo wrote:

I changed my mind a bit since my early comment you linked, having gone cold on the review and now having come back to it. Here's my thinking: the object manipulation performed by the objcopy code is based on an internal Object class that has nothing to do with the object classes within the libObject library. By putting the two inside the same library, we risk confusion ("which type of Object do I need for this functionality?"). There are fundamental differences which would make reusing the libObject classes for the objcopy code less than ideal - it may not even be possible without a lot of work.

The original idea is that these classes -could- be used for this. If that's not true we should go back and fix that rather than have a separate set. That said, we do have a separate set which complicates matters. I think the question I'd ask is "how can we get from here to there?". No straightforward answer here for sure.

I guess I might have misrepresented the internals of the llvm-objcopy classes: to populate them, they do use the libObject code. The llvm-objcopy Object class acts as another layer that provides manipulation functionality on top of the libObject object files. In some ways, it could actually be used purely for reading though, which raises some interesting thoughts which I can't exactly express. If we wanted to change the existing libObject code to support direct manipulation, without this extra layer, we'd need to change how those classes store data - at the moment, they just consist of references into memory buffers, rather than copies stored within the program. As such, it is impossible to change these references without changing how data is backed. The llvm-objcopy library provides this additional layer in essence. I think this shows that, at least at the moment, the llvm-objcopy Object code is a separate layer. Whethere that then means the separate layer should be in a separate library is probably a different point (it means it can be, but doesn't mean it must be.

Regarding the Archive writing code being in libObject: it's my tentative opinion that archive functionality shouldn't be in libObject at all: an archive isn't really an object itself: it's a group of objects. The functionality for archives is completely unrelated to the functionality for other file types supported by the object library. As such, I think it should actually be moved into a separate libArchive library or similar.

From my perspective: ideally an archive should just be a specific type of object that contains other objects (similar to perhaps a shared library if we did that). Reading it just means cracking it, sharing the archive format information, and then iterating through each object accordingly. Writing would be the reverse. Any thoughts on where we're on a different page? :)

I don't think we really disagree here (although I wouldn't call an archive an object in those words). The point I was largely addressing was the earlier point made by @alexander-shaposhnikov about how the object library already allows manipulation of archives, unlike the other object formats.

Finally, there's @sbc100's point earlier: whilst it's true that by having the code in the same library doesn't force tools to link in that code, it does require distributions to include the llvm-objcopy library code anytime it needs the libObject code, because it's part of that library, even though it may not need it. I've seen several instances internally where we generate packages of LLVM libraries, without including all of them, so including the llvm-objcopy code in the libObject library would bloat the size of the package.

Are you thinking about things just shipping .a or .o files or something else? Most platforms that I can think of can remove unneeded object code in a binary link so I'm curious about the concern here.

Yeah, we have some cases where we distribute .a files (or more specifically Windows .lib files, but the principle is the same). A couple of concrete examples to illustrate the points: firstly, if I want to build LLD, I don't need the llvm-objcopy code. Let's say llvm-objcopy included 5 files. If those 5 files are included in libObject, they need to be compiled and added to that library, as part fo the build process for building LLD, even though those files aren't used in LLD's link, thus unnecessarily slowing down the build as a whole. Secondly, if I have a distribution that wants to ship LLVM libraries sufficient for developers to read objects, we'd end up distributing those same 5 files as part of libObject, unnecessarily. The end executables won't be any bigger in either case of course.

@echristo, @alexander-shaposhnikov, do either of you have any further thoughts? What do you think of my points above? I'm not strongly opposed to it going in libObject, but just think it makes a little more sense to be separate. Also, would either of you be able to give this a review more generally?

I'll do my best :)

-eric

FYI, I'm on leave for the next 2-3 weeks, so won't be able to respond to further points after this for a while.

In D88827#3242580, @MaskRay wrote:

llvm-objcopy has its own data models for some binary format specific things, so placing it under the llvm::object namespace may lead to some conflicts. Placing it into a sub-namespace does not avoid the conflict.

Not sure I follow how placing things into, say, llvm::object::objcopy would have potential for conflicts beyond the current state of the code?

[avl]

In D88827#3243266, @jhenderson wrote:

In D88827#3242326, @echristo wrote:

Finally, there's @sbc100's point earlier: whilst it's true that by having the code in the same library doesn't force tools to link in that code, it does require distributions to include the llvm-objcopy library code anytime it needs the libObject code, because it's part of that library, even though it may not need it. I've seen several instances internally where we generate packages of LLVM libraries, without including all of them, so including the llvm-objcopy code in the libObject library would bloat the size of the package.

Are you thinking about things just shipping .a or .o files or something else? Most platforms that I can think of can remove unneeded object code in a binary link so I'm curious about the concern here.

Yeah, we have some cases where we distribute .a files (or more specifically Windows .lib files, but the principle is the same). A couple of concrete examples to illustrate the points: firstly, if I want to build LLD, I don't need the llvm-objcopy code. Let's say llvm-objcopy included 5 files. If those 5 files are included in libObject, they need to be compiled and added to that library, as part fo the build process for building LLD, even though those files aren't used in LLD's link, thus unnecessarily slowing down the build as a whole. Secondly, if I have a distribution that wants to ship LLVM libraries sufficient for developers to read objects, we'd end up distributing those same 5 files as part of libObject, unnecessarily. The end executables won't be any bigger in either case of course.

there is also a scenario when end executables would be bigger(though it is not about shipping, this is for debug builds). It is assumed that --gc-sections or similar will remove unused code and then the final executable would be small. That is not exactly true for debug builds. --gc-sections will not remove unused debuginfo. Thus end debug executables would contain unnecessary debuginfo and then will be bigger.

[jhenderson]

In D88827#3243279, @avl wrote:

In D88827#3243266, @jhenderson wrote:

In D88827#3242326, @echristo wrote:

Finally, there's @sbc100's point earlier: whilst it's true that by having the code in the same library doesn't force tools to link in that code, it does require distributions to include the llvm-objcopy library code anytime it needs the libObject code, because it's part of that library, even though it may not need it. I've seen several instances internally where we generate packages of LLVM libraries, without including all of them, so including the llvm-objcopy code in the libObject library would bloat the size of the package.

Are you thinking about things just shipping .a or .o files or something else? Most platforms that I can think of can remove unneeded object code in a binary link so I'm curious about the concern here.

Yeah, we have some cases where we distribute .a files (or more specifically Windows .lib files, but the principle is the same). A couple of concrete examples to illustrate the points: firstly, if I want to build LLD, I don't need the llvm-objcopy code. Let's say llvm-objcopy included 5 files. If those 5 files are included in libObject, they need to be compiled and added to that library, as part fo the build process for building LLD, even though those files aren't used in LLD's link, thus unnecessarily slowing down the build as a whole. Secondly, if I have a distribution that wants to ship LLVM libraries sufficient for developers to read objects, we'd end up distributing those same 5 files as part of libObject, unnecessarily. The end executables won't be any bigger in either case of course.

there is also a scenario when end executables would be bigger(though it is not about shipping, this is for debug builds). It is assumed that --gc-sections or similar will remove unused code and then the final executable would be small. That is not exactly true for debug builds. --gc-sections will not remove unused debuginfo. Thus end debug executables would contain unnecessary debuginfo and then will be bigger.

This isn't generally true: even in debug builds and without --gc-sections, linkers tend not to link in unused objects into the final executable.

[avl]

This isn't generally true: even in debug builds and without --gc-sections, linkers tend not to link in unused objects into the final executable.

I remember there was a patch to make it possible - https://reviews.llvm.org/D54747 . Which finally was not applied and current behavior that it is not working(if I did not miss something new).

[mstorsjo]

In D88827#3243298, @avl wrote:

This isn't generally true: even in debug builds and without --gc-sections, linkers tend not to link in unused objects into the final executable.

I remember there was a patch to make it possible - https://reviews.llvm.org/D54747 . Which finally was not applied and current behavior that it is not working(if I did not miss something new).

The difference here is that if you feed the linker a specific set of object files, it will include all of them (minus what --gc-sections removes). But if you feed the linker a static library, it will only include the object files that actually are referenced (unless the library is added with the --whole-archive option or something similar).

[avl]

In D88827#3243317, @mstorsjo wrote:

In D88827#3243298, @avl wrote:

This isn't generally true: even in debug builds and without --gc-sections, linkers tend not to link in unused objects into the final executable.

I remember there was a patch to make it possible - https://reviews.llvm.org/D54747 . Which finally was not applied and current behavior that it is not working(if I did not miss something new).

The difference here is that if you feed the linker a specific set of object files, it will include all of them (minus what --gc-sections removes). But if you feed the linker a static library, it will only include the object files that actually are referenced (unless the library is added with the --whole-archive option or something similar).

Ah, right. We are talking about library here.

[alexander-shaposhnikov]

@jhenderson accurately expressed my considerations / original reasoning. Sorry about the very late reply. Perhaps, it's worth adding a few words.
I kind of envisioned the following minimalistic interface exposed by the library (essentially just a single function):

Error copy(const Config &C, const object::Object &O, raw_ostream &Out)

My hope was that we would refactor Config significantly and clean it up with 2 major goals in mind: (1) have proper separation of concerns and structure: Config { ... <common options>, COFFConfig, ELFConfig, MachOConfig };
(in particular, the fields which are not required anymore would be removed) (2) the functionality which is specific to the tool (e.g. creating a .DWO file) would live in the tool and would not be a part of the library. The same applies e.g. to setting file attributes.
The question wether it should be a separate library of a part of libObject - I had mixed feelings, based on the interface I thought it was appropriate to have it in libObject, but making it a separate library also makes sense to me.

[avl]

@alexander-shaposhnikov My understaning is that current state of this patch is pretty close to above description:

I kind of envisioned the following minimalistic interface exposed by the library (essentially just a single function):

Error copy(const Config &C, const object::Object &O, raw_ostream &Out)

Error executeObjcopyOnBinary(const MultiFormatConfig &Config,
                             object::Binary &In, raw_ostream &Out);

This function does dispatch on input formats and calls format-specific function.

It is not a single function though. I think we may refactor it further with separate patches.

My hope was that we would refactor Config significantly and clean it up with 2 major goals in mind: (1) have proper separation of concerns and structure: Config { ... <common options>, COFFConfig, ELFConfig, MachOConfig };
(in particular, the fields which are not required anymore would be removed)

Config was refactored by following patches :

https://reviews.llvm.org/D99055
https://reviews.llvm.org/D102277
https://reviews.llvm.org/D103260

(2) the functionality which is specific to the tool (e.g. creating a .DWO file) would live in the tool and would not be a part of the library. The same applies e.g. to setting file attributes.

these patches did it possible to leave creating a .DWO file, setting attributes, creating streams inside the tool:

https://reviews.llvm.org/D98582
https://reviews.llvm.org/D98511
https://reviews.llvm.org/D91028
https://reviews.llvm.org/D98426
https://reviews.llvm.org/D95478

[avl]

@echristo Are you satisfied with response by @jhenderson ?

following is a reminder, if messages are lost in the history:

That is @echristo message - https://reviews.llvm.org/D88827#3242326
That is @jhenderson response - https://reviews.llvm.org/D88827#3243266

[avl]

move implementation into the separate library ObjCopy.

[jhenderson]

LGTM, with two nits.

llvm/include/llvm/ObjCopy/ObjCopy.h

22–27 ↗

(On Diff #409152)

This doesn't need to be in the header, right? I'd get rid of it, as it's not really part fo the public interface (executeObjcopyOnArchive would be the right thing to use, I believe(?)).

llvm/tools/llvm-objcopy/llvm-objcopy.cpp

16

Delete this blank line then resort the headers.

[avl]

llvm/include/llvm/ObjCopy/ObjCopy.h

22–27 ↗

(On Diff #409152)

This also is used in lib/ObjCopy/MachO/MachOObjcopy.cpp. But, indeed, it might be removed from the public interface - include/llvm/ObjCopy/ObjCopy.h. Would it be OK if declaration of createNewArchiveMembers would be put into the new local header llvm/lib/ObjCopy/Archive.h ?

[jhenderson]

llvm/include/llvm/ObjCopy/ObjCopy.h

22–27 ↗

(On Diff #409152)

Yes, I think that should be okay.

[avl]

addressed comments - moved createNewArchiveMembers into separate header.

[avl]

@jhenderson Thank you for the review!

[thakis]

As far as I know, you also have to update the entries in clang/docs/tools/clang-formatted-files.txt.

llvm/lib/CMakeLists.txt

22

Can we be consistent about capitalization of "ObjCopy" or "Objcopy"? The new lib capitalizes the C, but e.g. ObjcopyOptions.cpp doesn't. (Don't care which way, just be consistent).

% git grep Objcopy | wc -l
     150
% git grep ObjCopy | wc -l
      77

llvm/unittests/ObjCopy/CMakeLists.txt

2

I think you have to add_subdirectory this in llvm/unittests/CMakeLists.txt to get this test binary actually built.

[avl]

@thakis Thanks! will implement comments with followup commit.

[avl]

@thakis addressed comments in https://reviews.llvm.org/rG0b57e6c46b707c0e7a123efe82abf3c1e7b5a503. As to the consistent naming - will do it in the separate patch.

[thakis]

I'm also seeing a bunch of warnings that look related:

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning same member name (ObjCopy.Object.o) in output file used for input files: obj/llvm/lib/ObjCopy/ELF/ObjCopy.Object.o and: obj/llvm/lib/ObjCopy/COFF/ObjCopy.Object.o (due to use of basename, truncation, blank padding or duplicate input files)
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning same member name (ObjCopy.Object.o) in output file used for input files: obj/llvm/lib/ObjCopy/COFF/ObjCopy.Object.o and: obj/llvm/lib/ObjCopy/MachO/ObjCopy.Object.o (due to use of basename, truncation, blank padding or duplicate input files)
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning same member name (ObjCopy.Object.o) in output file used for input files: obj/llvm/lib/ObjCopy/MachO/ObjCopy.Object.o and: obj/llvm/lib/ObjCopy/wasm/ObjCopy.Object.o (due to use of basename, truncation, blank padding or duplicate input files)
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning same member name (ObjCopy.Reader.o) in output file used for input files: obj/llvm/lib/ObjCopy/COFF/ObjCopy.Reader.o and: obj/llvm/lib/ObjCopy/wasm/ObjCopy.Reader.o (due to use of basename, truncation, blank padding or duplicate input files)
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning same member name (ObjCopy.Writer.o) in output file used for input files: obj/llvm/lib/ObjCopy/wasm/ObjCopy.Writer.o and: obj/llvm/lib/ObjCopy/COFF/ObjCopy.Writer.o (due to use of basename, truncation, blank padding or duplicate input files)

[avl]

In D88827#3335814, @thakis wrote:

I'm also seeing a bunch of warnings that look related:

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning same member name (ObjCopy.Object.o) in output file used for input files: obj/llvm/lib/ObjCopy/ELF/ObjCopy.Object.o and: obj/llvm/lib/ObjCopy/COFF/ObjCopy.Object.o (due to use of basename, truncation, blank padding or duplicate input files)
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning same member name (ObjCopy.Object.o) in output file used for input files: obj/llvm/lib/ObjCopy/COFF/ObjCopy.Object.o and: obj/llvm/lib/ObjCopy/MachO/ObjCopy.Object.o (due to use of basename, truncation, blank padding or duplicate input files)
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning same member name (ObjCopy.Object.o) in output file used for input files: obj/llvm/lib/ObjCopy/MachO/ObjCopy.Object.o and: obj/llvm/lib/ObjCopy/wasm/ObjCopy.Object.o (due to use of basename, truncation, blank padding or duplicate input files)
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning same member name (ObjCopy.Reader.o) in output file used for input files: obj/llvm/lib/ObjCopy/COFF/ObjCopy.Reader.o and: obj/llvm/lib/ObjCopy/wasm/ObjCopy.Reader.o (due to use of basename, truncation, blank padding or duplicate input files)
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning same member name (ObjCopy.Writer.o) in output file used for input files: obj/llvm/lib/ObjCopy/wasm/ObjCopy.Writer.o and: obj/llvm/lib/ObjCopy/COFF/ObjCopy.Writer.o (due to use of basename, truncation, blank padding or duplicate input files)

My understanding is that it is not a problem for linking. But it may be a problem if one would want to extract the files from libLLVMObjCopy.a. To avoid this problem we need to rename files. Will do it with separate patch.

llvm/include/llvm/ObjCopy/Buffer.h

 //===- Buffer.h -------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TOOLS_OBJCOPY_BUFFER_H
-#define LLVM_TOOLS_OBJCOPY_BUFFER_H
+#ifndef LLVM_OBJCOPY_BUFFER_H
+#define LLVM_OBJCOPY_BUFFER_H
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include <memory>
 
 namespace llvm {
 namespace objcopy {
 
 // The class Buffer abstracts out the common interface of FileOutputBuffer and
 // WritableMemoryBuffer so that the hierarchy of Writers depends on this
 // abstract interface and doesn't depend on a particular implementation.
 // TODO: refactor the buffer classes in LLVM to enable us to use them here
 // directly.
 class Buffer {
   StringRef Name;
 
 public:
   virtual ~Buffer();
   virtual Error allocate(size_t Size) = 0;
   virtual uint8_t *getBufferStart() = 0;
   virtual Error commit() = 0;
public:
   explicit MemBuffer(StringRef Name) : Buffer(Name) {}
 
   std::unique_ptr<WritableMemoryBuffer> releaseMemoryBuffer();
 };
 
 } // end namespace objcopy
 } // end namespace llvm
 
-#endif // LLVM_TOOLS_OBJCOPY_BUFFER_H
+#endif // LLVM_OBJCOPY_BUFFER_H

llvm/include/llvm/ObjCopy/COFF/COFFObjcopy.h

 //===- COFFObjcopy.h --------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TOOLS_OBJCOPY_COFFOBJCOPY_H
-#define LLVM_TOOLS_OBJCOPY_COFFOBJCOPY_H
+#ifndef LLVM_OBJCOPY_COFF_COFFOBJCOPY_H
+#define LLVM_OBJCOPY_COFF_COFFOBJCOPY_H
 
 namespace llvm {
 class Error;
 
 namespace object {
 class COFFObjectFile;
 } // end namespace object
 
 namespace objcopy {
 struct CopyConfig;
 class Buffer;
 
 namespace coff {
 Error executeObjcopyOnBinary(const CopyConfig &Config,
                              object::COFFObjectFile &In, Buffer &Out);
 
 } // end namespace coff
 } // end namespace objcopy
 } // end namespace llvm
 
-#endif // LLVM_TOOLS_OBJCOPY_COFFOBJCOPY_H
+#endif // LLVM_OBJCOPY_COFF_COFFOBJCOPY_H

llvm/include/llvm/ObjCopy/COFF/Object.h

 //===- Object.h -------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TOOLS_OBJCOPY_COFF_OBJECT_H
-#define LLVM_TOOLS_OBJCOPY_COFF_OBJECT_H
+#ifndef LLVM_OBJCOPY_COFF_OBJECT_H
+#define LLVM_OBJCOPY_COFF_OBJECT_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/BinaryFormat/COFF.h"
 #include "llvm/Object/COFF.h"
 #include <cstddef>
 #include <cstdint>
 #include <vector>
 
 namespace llvm {
 namespace objcopy {
 namespace coff {
 
 struct Relocation {
   Relocation() = default;
-  Relocation(const object::coff_relocation& R) : Reloc(R) {}
+  Relocation(const object::coff_relocation &R) : Reloc(R) {}
 
   object::coff_relocation Reloc;
   size_t Target = 0;
   StringRef TargetName; // Used for diagnostics only
 };
 
 struct Section {
   object::coff_section Header;
void copyPeHeader(PeHeader1Ty &Dest, const PeHeader2Ty &Src) {
   Dest.LoaderFlags = Src.LoaderFlags;
   Dest.NumberOfRvaAndSize = Src.NumberOfRvaAndSize;
 }
 
 } // end namespace coff
 } // end namespace objcopy
 } // end namespace llvm
 
-#endif // LLVM_TOOLS_OBJCOPY_COFF_OBJECT_H
+#endif // LLVM_OBJCOPY_COFF_OBJECT_H

llvm/include/llvm/ObjCopy/COFF/Reader.h

 //===- Reader.h -------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TOOLS_OBJCOPY_COFF_READER_H
-#define LLVM_TOOLS_OBJCOPY_COFF_READER_H
+#ifndef LLVM_OBJCOPY_COFF_READER_H
+#define LLVM_OBJCOPY_COFF_READER_H
 
-#include "Buffer.h"
 #include "llvm/BinaryFormat/COFF.h"
+#include "llvm/ObjCopy/Buffer.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Support/Error.h"
 
 namespace llvm {
 namespace objcopy {
 namespace coff {
 
 struct Object;
public:
   explicit COFFReader(const COFFObjectFile &O) : COFFObj(O) {}
   Expected<std::unique_ptr<Object>> create() const;
 };
 
 } // end namespace coff
 } // end namespace objcopy
 } // end namespace llvm
 
-#endif // LLVM_TOOLS_OBJCOPY_COFF_READER_H
+#endif // LLVM_OBJCOPY_COFF_READER_H

llvm/include/llvm/ObjCopy/COFF/Writer.h

 //===- Writer.h -------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TOOLS_OBJCOPY_COFF_WRITER_H
-#define LLVM_TOOLS_OBJCOPY_COFF_WRITER_H
+#ifndef LLVM_OBJCOPY_COFF_WRITER_H
+#define LLVM_OBJCOPY_COFF_WRITER_H
 
-#include "Buffer.h"
 #include "llvm/MC/StringTableBuilder.h"
+#include "llvm/ObjCopy/Buffer.h"
 #include "llvm/Support/Error.h"
 #include <cstddef>
 #include <utility>
 
 namespace llvm {
 namespace objcopy {
 namespace coff {
 
public:
   COFFWriter(Object &Obj, Buffer &Buf)
       : Obj(Obj), Buf(Buf), StrTabBuilder(StringTableBuilder::WinCOFF) {}
 };
 
 } // end namespace coff
 } // end namespace objcopy
 } // end namespace llvm
 
-#endif // LLVM_TOOLS_OBJCOPY_COFF_WRITER_H
+#endif // LLVM_OBJCOPY_COFF_WRITER_H

llvm/include/llvm/ObjCopy/CopyConfig.h

 //===- CopyConfig.h -------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TOOLS_LLVM_OBJCOPY_COPY_CONFIG_H
-#define LLVM_TOOLS_LLVM_OBJCOPY_COPY_CONFIG_H
+#ifndef LLVM_OBJCOPY_COPY_CONFIG_H
+#define LLVM_OBJCOPY_COPY_CONFIG_H
 
 #include "ELF/ELFConfig.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitmaskEnum.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
public:
 
   bool isPositiveMatch() const { return IsPositiveMatch; }
   bool operator==(StringRef S) const {
     return R ? R->match(S) : G ? G->match(S) : Name == S;
   }
   bool operator!=(StringRef S) const { return !operator==(S); }
 };
 
 // Matcher that checks symbol or section names against the command line flags
 // provided for that option.
 class NameMatcher {
   std::vector<NameOrPattern> PosMatchers;
   std::vector<NameOrPattern> NegMatchers;
 
 public:
   Error addMatcher(Expected<NameOrPattern> Matcher) {
     if (!Matcher)
       return Matcher.takeError();
if (!ELF) {
       if (!ELFConfig)
         return ELFConfig.takeError();
       ELF = *ELFConfig;
     }
     return Error::success();
   }
 };
 
-// Configuration for the overall invocation of this tool. When invoked as
-// objcopy, will always contain exactly one CopyConfig. When invoked as strip,
-// will contain one or more CopyConfigs.
-struct DriverConfig {
-  SmallVector<CopyConfig, 1> CopyConfigs;
-  BumpPtrAllocator Alloc;
-};
-
-// ParseObjcopyOptions returns the config and sets the input arguments. If a
-// help flag is set then ParseObjcopyOptions will print the help messege and
-// exit. ErrorCallback is used to handle recoverable errors. An Error returned
-// by the callback aborts the parsing and is then returned by this function.
-Expected<DriverConfig>
-parseObjcopyOptions(ArrayRef<const char *> ArgsArr,
-                    llvm::function_ref<Error(Error)> ErrorCallback);
-
-// ParseInstallNameToolOptions returns the config and sets the input arguments.
-// If a help flag is set then ParseInstallNameToolOptions will print the help
-// messege and exit.
-Expected<DriverConfig>
-parseInstallNameToolOptions(ArrayRef<const char *> ArgsArr);
-
-// ParseBitcodeStripOptions returns the config and sets the input arguments.
-// If a help flag is set then ParseBitcodeStripOptions will print the help
-// messege and exit.
-Expected<DriverConfig> parseBitcodeStripOptions(ArrayRef<const char *> ArgsArr);
-
-// ParseStripOptions returns the config and sets the input arguments. If a
-// help flag is set then ParseStripOptions will print the help messege and
-// exit. ErrorCallback is used to handle recoverable errors. An Error returned
-// by the callback aborts the parsing and is then returned by this function.
-Expected<DriverConfig>
-parseStripOptions(ArrayRef<const char *> ArgsArr,
-                  llvm::function_ref<Error(Error)> ErrorCallback);
 } // namespace objcopy
 } // namespace llvm
 
-#endif
+#endif // LLVM_OBJCOPY_COPY_CONFIG_H

llvm/include/llvm/ObjCopy/ELF/ELFConfig.h

 //===- ELFConfig.h ----------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TOOLS_OBJCOPY_ELFCONFIG_H
-#define LLVM_TOOLS_OBJCOPY_ELFCONFIG_H
+#ifndef LLVM_OBJCOPY_ELF_ELFCONFIG_H
+#define LLVM_OBJCOPY_ELF_ELFCONFIG_H
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Object/ELFTypes.h"
 #include "llvm/Support/Error.h"
 #include <vector>
 
 namespace llvm {

 };
 
 Expected<ELFCopyConfig> parseConfig(const CopyConfig &Config);
 
 } // namespace elf
 } // namespace objcopy
 } // namespace llvm
 
-#endif
+#endif // LLVM_OBJCOPY_ELF_ELFCONFIG_H

llvm/include/llvm/ObjCopy/ELF/ELFObjcopy.h

 //===- ELFObjcopy.h ---------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TOOLS_OBJCOPY_ELFOBJCOPY_H
-#define LLVM_TOOLS_OBJCOPY_ELFOBJCOPY_H
+#ifndef LLVM_OBJCOPY_ELF_ELFOBJCOPY_H
+#define LLVM_OBJCOPY_ELF_ELFOBJCOPY_H
 
 namespace llvm {
 class Error;
 class MemoryBuffer;
 
 namespace object {
 class ELFObjectFileBase;
 } // end namespace object
Error executeObjcopyOnRawBinary(const CopyConfig &Config, MemoryBuffer &In,
                                 Buffer &Out);
 Error executeObjcopyOnBinary(const CopyConfig &Config,
                              object::ELFObjectFileBase &In, Buffer &Out);
 
 } // end namespace elf
 } // end namespace objcopy
 } // end namespace llvm
 
-#endif // LLVM_TOOLS_OBJCOPY_ELFOBJCOPY_H
+#endif // LLVM_OBJCOPY_ELF_ELFOBJCOPY_H

llvm/include/llvm/ObjCopy/ELF/Object.h

 //===- Object.h -------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TOOLS_OBJCOPY_OBJECT_H
-#define LLVM_TOOLS_OBJCOPY_OBJECT_H
+#ifndef LLVM_OBJCOPY_ELF_OBJECT_H
+#define LLVM_OBJCOPY_ELF_OBJECT_H
 
-#include "Buffer.h"
-#include "CopyConfig.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/MC/StringTableBuilder.h"
+#include "llvm/ObjCopy/Buffer.h"
+#include "llvm/ObjCopy/CopyConfig.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include <cstddef>
 #include <cstdint>
 #include <functional>
 #include <memory>
 #include <set>
class Section : public SectionBase {
   ArrayRef<uint8_t> Contents;
   SectionBase *LinkSection = nullptr;
 
 public:
   explicit Section(ArrayRef<uint8_t> Data) : Contents(Data) {}
 
   Error accept(SectionVisitor &Visitor) const override;
   Error accept(MutableSectionVisitor &Visitor) override;
-  Error removeSectionReferences(bool AllowBrokenLinks,
+  Error removeSectionReferences(
+      bool AllowBrokenLinks,
       function_ref<bool(const SectionBase *)> ToRemove) override;
   Error initialize(SectionTableRef SecTable) override;
   void finalize() override;
 };
 
 class OwnedDataSection : public SectionBase {
   MAKE_SEC_WRITER_FRIEND
 

 private:
   std::vector<uint32_t> Indexes;
   SymbolTableSection *Symbols = nullptr;
 
 public:
   virtual ~SectionIndexSection() {}
   void addIndex(uint32_t Index) {
     assert(Size > 0);
-    Indexes.push_back(Index);    
+    Indexes.push_back(Index);
   }
 
   void reserve(size_t NumSymbols) {
     Indexes.reserve(NumSymbols);
     Size = NumSymbols * 4;
-  }  
+  }
   void setSymTab(SymbolTableSection *SymTab) { Symbols = SymTab; }
   Error initialize(SectionTableRef SecTable) override;
   void finalize() override;
   Error accept(SectionVisitor &Visitor) const override;
   Error accept(MutableSectionVisitor &Visitor) override;
 
   SectionIndexSection() {
     Name = ".symtab_shndx";
public:
   }
   const SectionIndexSection *getShndxTable() const { return SectionIndexTable; }
   void fillShndxTable();
   const SectionBase *getStrTab() const { return SymbolNames; }
   Expected<const Symbol *> getSymbolByIndex(uint32_t Index) const;
   Expected<Symbol *> getSymbolByIndex(uint32_t Index);
   void updateSymbols(function_ref<void(Symbol &)> Callable);
 
-  Error removeSectionReferences(bool AllowBrokenLinks,
+  Error removeSectionReferences(
+      bool AllowBrokenLinks,
       function_ref<bool(const SectionBase *)> ToRemove) override;
   Error initialize(SectionTableRef SecTable) override;
   void finalize() override;
   Error accept(SectionVisitor &Visitor) const override;
   Error accept(MutableSectionVisitor &Visitor) override;
   Error removeSymbols(function_ref<bool(const Symbol &)> ToRemove) override;
   void replaceSectionReferences(
       const DenseMap<SectionBase *, SectionBase *> &FromTo) override;
class RelocationSection
   MAKE_SEC_WRITER_FRIEND
 
   std::vector<Relocation> Relocations;
 
 public:
   void addRelocation(Relocation Rel) { Relocations.push_back(Rel); }
   Error accept(SectionVisitor &Visitor) const override;
   Error accept(MutableSectionVisitor &Visitor) override;
-  Error removeSectionReferences(bool AllowBrokenLinks,
+  Error removeSectionReferences(
+      bool AllowBrokenLinks,
       function_ref<bool(const SectionBase *)> ToRemove) override;
   Error removeSymbols(function_ref<bool(const Symbol &)> ToRemove) override;
   void markSymbols() override;
   void replaceSectionReferences(
       const DenseMap<SectionBase *, SectionBase *> &FromTo) override;
 
   static bool classof(const SectionBase *S) {
     if (S->OriginalFlags & ELF::SHF_ALLOC)
bool isRelocatable() const {
     return (Type != ELF::ET_DYN && Type != ELF::ET_EXEC) || MustBeRelocatable;
   }
 };
 
 } // end namespace elf
 } // end namespace objcopy
 } // end namespace llvm
 
-#endif // LLVM_TOOLS_OBJCOPY_OBJECT_H
+#endif // LLVM_OBJCOPY_ELF_OBJECT_H

llvm/include/llvm/ObjCopy/MachO/MachOLayoutBuilder.h

 //===- MachOLayoutBuilder.h -------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_OBJCOPY_MACHO_MACHOLAYOUTBUILDER_H
 #define LLVM_OBJCOPY_MACHO_MACHOLAYOUTBUILDER_H
 
 #include "MachOObjcopy.h"
 #include "Object.h"
 
 namespace llvm {
 namespace objcopy {
 namespace macho {
 
 class MachOLayoutBuilder {
   Object &O;
   bool Is64Bit;
   uint64_t PageSize;
 
   // Points to the __LINKEDIT segment if it exists.
   MachO::macho_load_command *LinkEditLoadCommand = nullptr;
   StringTableBuilder StrTableBuilder{StringTableBuilder::MachO};
 
   uint32_t computeSizeOfCmds() const;
   void constructStringTable();
   void updateSymbolIndexes();
   void updateDySymTab(MachO::macho_load_command &MLC);
   uint64_t layoutSegments();
   uint64_t layoutRelocations(uint64_t Offset);
   Error layoutTail(uint64_t Offset);
 
 public:
   MachOLayoutBuilder(Object &O, bool Is64Bit, uint64_t PageSize)
       : O(O), Is64Bit(Is64Bit), PageSize(PageSize) {}
 
   // Recomputes and updates fields in the given object such as file offsets.
   Error layout();
 
   StringTableBuilder &getStringTableBuilder() { return StrTableBuilder; }
 };
 
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm
 
 #endif // LLVM_OBJCOPY_MACHO_MACHOLAYOUTBUILDER_H

llvm/include/llvm/ObjCopy/MachO/MachOObjcopy.h

 //===- MachOObjcopy.h -------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TOOLS_OBJCOPY_MACHOOBJCOPY_H
-#define LLVM_TOOLS_OBJCOPY_MACHOOBJCOPY_H
+#ifndef LLVM_OBJCOPY_MACHO_MACHOOBJCOPY_H
+#define LLVM_OBJCOPY_MACHO_MACHOOBJCOPY_H
 
 namespace llvm {
 class Error;
 
 namespace object {
 class MachOObjectFile;
 class MachOUniversalBinary;
 } // end namespace object
 
 namespace objcopy {
 struct CopyConfig;
 class Buffer;
 
 namespace macho {
 Error executeObjcopyOnBinary(const CopyConfig &Config,
                              object::MachOObjectFile &In, Buffer &Out);
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm
 
-#endif // LLVM_TOOLS_OBJCOPY_MACHOOBJCOPY_H
+#endif // LLVM_OBJCOPY_MACHO_MACHOOBJCOPY_H

llvm/include/llvm/ObjCopy/MachO/MachOReader.h

 //===- MachOReader.h --------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
+#ifndef LLVM_OBJCOPY_MACHO_MACHOREADER_H
+#define LLVM_OBJCOPY_MACHO_MACHOREADER_H
+
 #include "MachOObjcopy.h"
 #include "Object.h"
 #include "llvm/BinaryFormat/MachO.h"
 #include "llvm/Object/MachO.h"
 #include <memory>
 
 namespace llvm {
 namespace objcopy {
public:
   explicit MachOReader(const object::MachOObjectFile &Obj) : MachOObj(Obj) {}
 
   Expected<std::unique_ptr<Object>> create() const override;
 };
 
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm
+
+#endif // LLVM_OBJCOPY_MACHO_MACHOREADER_H

llvm/include/llvm/ObjCopy/MachO/MachOWriter.h

 //===- MachOWriter.h --------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "../Buffer.h"
+#ifndef LLVM_OBJCOPY_MACHO_MACHOWRITER_H
+#define LLVM_OBJCOPY_MACHO_MACHOWRITER_H
+
 #include "MachOLayoutBuilder.h"
 #include "MachOObjcopy.h"
 #include "Object.h"
 #include "llvm/BinaryFormat/MachO.h"
+#include "llvm/ObjCopy/Buffer.h"
 #include "llvm/Object/MachO.h"
 
 namespace llvm {
 class Error;
 
 namespace objcopy {
 namespace macho {
 
public:
   size_t totalSize() const;
   Error finalize();
   Error write();
 };
 
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm
+
+#endif // LLVM_OBJCOPY_MACHO_MACHOWRITER_H

llvm/include/llvm/ObjCopy/MachO/Object.h

 //===- Object.h - Mach-O object file model ----------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_OBJCOPY_MACHO_OBJECT_H
 #define LLVM_OBJCOPY_MACHO_OBJECT_H
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/MachO.h"
 #include "llvm/MC/StringTableBuilder.h"
 #include "llvm/ObjectYAML/DWARFYAML.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/YAMLTraits.h"
 #include <cstdint>
 #include <string>
 #include <vector>
 
 namespace llvm {
 namespace objcopy {
 namespace macho {
 
 struct MachHeader {
   uint32_t Magic;
   uint32_t CPUType;
   uint32_t CPUSubType;
   uint32_t FileType;
   uint32_t NCmds;
   uint32_t SizeOfCmds;
   uint32_t Flags;
   uint32_t Reserved = 0;
 };
 
 struct RelocationInfo;
 struct Section {
   uint32_t Index;
   std::string Segname;
   std::string Sectname;
   // CanonicalName is a string formatted as “<Segname>,<Sectname>".
   std::string CanonicalName;
   uint64_t Addr = 0;
   uint64_t Size = 0;
   uint32_t Offset = 0;
   uint32_t Align = 0;
   uint32_t RelOff = 0;
   uint32_t NReloc = 0;
   uint32_t Flags = 0;
   uint32_t Reserved1 = 0;
   uint32_t Reserved2 = 0;
   uint32_t Reserved3 = 0;
   StringRef Content;
   std::vector<RelocationInfo> Relocations;
 
   Section(StringRef SegName, StringRef SectName)
       : Segname(std::string(SegName)), Sectname(std::string(SectName)),
         CanonicalName((Twine(SegName) + Twine(',') + SectName).str()) {}
 
   Section(StringRef SegName, StringRef SectName, StringRef Content)
       : Segname(std::string(SegName)), Sectname(std::string(SectName)),
         CanonicalName((Twine(SegName) + Twine(',') + SectName).str()),
         Content(Content) {}
 
   MachO::SectionType getType() const {
     return static_cast<MachO::SectionType>(Flags & MachO::SECTION_TYPE);
   }
 
   bool isVirtualSection() const {
     return (getType() == MachO::S_ZEROFILL ||
             getType() == MachO::S_GB_ZEROFILL ||
             getType() == MachO::S_THREAD_LOCAL_ZEROFILL);
   }
 };
 
 struct LoadCommand {
   // The type MachO::macho_load_command is defined in llvm/BinaryFormat/MachO.h
   // and it is a union of all the structs corresponding to various load
   // commands.
   MachO::macho_load_command MachOLoadCommand;
 
   // The raw content of the payload of the load command (located right after the
   // corresponding struct). In some cases it is either empty or can be
   // copied-over without digging into its structure.
   std::vector<uint8_t> Payload;
 
   // Some load commands can contain (inside the payload) an array of sections,
   // though the contents of the sections are stored separately. The struct
   // Section describes only sections' metadata and where to find the
   // corresponding content inside the binary.
   std::vector<std::unique_ptr<Section>> Sections;
 
   // Returns the segment name if the load command is a segment command.
   Optional<StringRef> getSegmentName() const;
 
   // Returns the segment vm address if the load command is a segment command.
   Optional<uint64_t> getSegmentVMAddr() const;
 };
 
 // A symbol information. Fields which starts with "n_" are same as them in the
 // nlist.
 struct SymbolEntry {
   std::string Name;
   bool Referenced = false;
   uint32_t Index;
   uint8_t n_type;
   uint8_t n_sect;
   uint16_t n_desc;
   uint64_t n_value;
 
   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 {
   std::vector<std::unique_ptr<SymbolEntry>> Symbols;
 
   using iterator = pointee_iterator<
       std::vector<std::unique_ptr<SymbolEntry>>::const_iterator>;
 
   iterator begin() const { return iterator(Symbols.begin()); }
   iterator end() const { return iterator(Symbols.end()); }
 
   const SymbolEntry *getSymbolByIndex(uint32_t Index) const;
   SymbolEntry *getSymbolByIndex(uint32_t Index);
   void removeSymbols(
       function_ref<bool(const std::unique_ptr<SymbolEntry> &)> ToRemove);
 };
 
 struct IndirectSymbolEntry {
   // The original value in an indirect symbol table. Higher bits encode extra
   // information (INDIRECT_SYMBOL_LOCAL and INDIRECT_SYMBOL_ABS).
   uint32_t OriginalIndex;
   /// The Symbol referenced by this entry. It's None if the index is
   /// INDIRECT_SYMBOL_LOCAL or INDIRECT_SYMBOL_ABS.
   Optional<SymbolEntry *> Symbol;
 
   IndirectSymbolEntry(uint32_t OriginalIndex, Optional<SymbolEntry *> Symbol)
       : OriginalIndex(OriginalIndex), Symbol(Symbol) {}
 };
 
 struct IndirectSymbolTable {
   std::vector<IndirectSymbolEntry> Symbols;
 };
 
 /// The location of the string table inside the binary is described by LC_SYMTAB
 /// load command.
 struct StringTable {
   std::vector<std::string> Strings;
 };
 
 struct RelocationInfo {
   // The referenced symbol entry. Set if !Scattered && Extern.
   Optional<const SymbolEntry *> Symbol;
   // The referenced section. Set if !Scattered && !Extern.
   Optional<const Section *> Sec;
   // True if Info is a scattered_relocation_info.
   bool Scattered;
   // True if the r_symbolnum points to a section number (i.e. r_extern=0).
   bool Extern;
   MachO::any_relocation_info Info;
 
   unsigned getPlainRelocationSymbolNum(bool IsLittleEndian) {
     if (IsLittleEndian)
       return Info.r_word1 & 0xffffff;
     return Info.r_word1 >> 8;
   }
 
   void setPlainRelocationSymbolNum(unsigned SymbolNum, bool IsLittleEndian) {
     assert(SymbolNum < (1 << 24) && "SymbolNum out of range");
     if (IsLittleEndian)
       Info.r_word1 = (Info.r_word1 & ~0x00ffffff) | SymbolNum;
     else
       Info.r_word1 = (Info.r_word1 & ~0xffffff00) | (SymbolNum << 8);
   }
 };
 
 /// The location of the rebase info inside the binary is described by
 /// LC_DYLD_INFO load command. Dyld rebases an image whenever dyld loads it at
 /// an address different from its preferred address.  The rebase information is
 /// a stream of byte sized opcodes whose symbolic names start with
 /// REBASE_OPCODE_. Conceptually the rebase information is a table of tuples:
 ///   <seg-index, seg-offset, type>
 /// The opcodes are a compressed way to encode the table by only
 /// encoding when a column changes.  In addition simple patterns
 /// like "every n'th offset for m times" can be encoded in a few
 /// bytes.
 struct RebaseInfo {
   // At the moment we do not parse this info (and it is simply copied over),
   // but the proper support will be added later.
   ArrayRef<uint8_t> Opcodes;
 };
 
 /// The location of the bind info inside the binary is described by
 /// LC_DYLD_INFO load command. Dyld binds an image during the loading process,
 /// if the image requires any pointers to be initialized to symbols in other
 /// images. The bind information is a stream of byte sized opcodes whose
 /// symbolic names start with BIND_OPCODE_. Conceptually the bind information is
 /// a table of tuples: <seg-index, seg-offset, type, symbol-library-ordinal,
 /// symbol-name, addend> The opcodes are a compressed way to encode the table by
 /// only encoding when a column changes.  In addition simple patterns like for
 /// runs of pointers initialized to the same value can be encoded in a few
 /// bytes.
 struct BindInfo {
   // At the moment we do not parse this info (and it is simply copied over),
   // but the proper support will be added later.
   ArrayRef<uint8_t> Opcodes;
 };
 
 /// The location of the weak bind info inside the binary is described by
 /// LC_DYLD_INFO load command. Some C++ programs require dyld to unique symbols
 /// so that all images in the process use the same copy of some code/data. This
 /// step is done after binding. The content of the weak_bind info is an opcode
 /// stream like the bind_info.  But it is sorted alphabetically by symbol name.
 /// This enable dyld to walk all images with weak binding information in order
 /// and look for collisions.  If there are no collisions, dyld does no updating.
 /// That means that some fixups are also encoded in the bind_info.  For
 /// instance, all calls to "operator new" are first bound to libstdc++.dylib
 /// using the information in bind_info.  Then if some image overrides operator
 /// new that is detected when the weak_bind information is processed and the
 /// call to operator new is then rebound.
 struct WeakBindInfo {
   // At the moment we do not parse this info (and it is simply copied over),
   // but the proper support will be added later.
   ArrayRef<uint8_t> Opcodes;
 };
 
 /// The location of the lazy bind info inside the binary is described by
 /// LC_DYLD_INFO load command. Some uses of external symbols do not need to be
 /// bound immediately. Instead they can be lazily bound on first use.  The
 /// lazy_bind contains a stream of BIND opcodes to bind all lazy symbols. Normal
 /// use is that dyld ignores the lazy_bind section when loading an image.
 /// Instead the static linker arranged for the lazy pointer to initially point
 /// to a helper function which pushes the offset into the lazy_bind area for the
 /// symbol needing to be bound, then jumps to dyld which simply adds the offset
 /// to lazy_bind_off to get the information on what to bind.
 struct LazyBindInfo {
   ArrayRef<uint8_t> Opcodes;
 };
 
 /// The location of the export info inside the binary is described by
 /// LC_DYLD_INFO load command. The symbols exported by a dylib are encoded in a
 /// trie.  This is a compact representation that factors out common prefixes. It
 /// also reduces LINKEDIT pages in RAM because it encodes all information (name,
 /// address, flags) in one small, contiguous range. The export area is a stream
 /// of nodes.  The first node sequentially is the start node for the trie. Nodes
 /// for a symbol start with a uleb128 that is the length of the exported symbol
 /// information for the string so far. If there is no exported symbol, the node
 /// starts with a zero byte. If there is exported info, it follows the length.
 /// First is a uleb128 containing flags. Normally, it is followed by
 /// a uleb128 encoded offset which is location of the content named
 /// by the symbol from the mach_header for the image.  If the flags
 /// is EXPORT_SYMBOL_FLAGS_REEXPORT, then following the flags is
 /// a uleb128 encoded library ordinal, then a zero terminated
 /// UTF8 string.  If the string is zero length, then the symbol
 /// is re-export from the specified dylib with the same name.
 /// If the flags is EXPORT_SYMBOL_FLAGS_STUB_AND_RESOLVER, then following
 /// the flags is two uleb128s: the stub offset and the resolver offset.
 /// The stub is used by non-lazy pointers.  The resolver is used
 /// by lazy pointers and must be called to get the actual address to use.
 /// After the optional exported symbol information is a byte of
 /// how many edges (0-255) that this node has leaving it,
 /// followed by each edge.
 /// Each edge is a zero terminated UTF8 of the addition chars
 /// in the symbol, followed by a uleb128 offset for the node that
 /// edge points to.
 struct ExportInfo {
   ArrayRef<uint8_t> Trie;
 };
 
 struct LinkData {
   ArrayRef<uint8_t> Data;
 };
 
 struct Object {
   MachHeader Header;
   std::vector<LoadCommand> LoadCommands;
 
   SymbolTable SymTable;
   StringTable StrTable;
 
   RebaseInfo Rebases;
   BindInfo Binds;
   WeakBindInfo WeakBinds;
   LazyBindInfo LazyBinds;
   ExportInfo Exports;
   IndirectSymbolTable IndirectSymTable;
   LinkData DataInCode;
   LinkData FunctionStarts;
   LinkData CodeSignature;
 
   Optional<uint32_t> SwiftVersion;
 
   /// The index of LC_CODE_SIGNATURE load command if present.
   Optional<size_t> CodeSignatureCommandIndex;
   /// 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;
   /// The index LC_FUNCTION_STARTS load comamnd if present.
   Optional<size_t> FunctionStartsCommandIndex;
 
   BumpPtrAllocator Alloc;
   StringSaver NewSectionsContents;
 
   Object() : NewSectionsContents(Alloc) {}
 
   Error
   removeSections(function_ref<bool(const std::unique_ptr<Section> &)> ToRemove);
 
   Error removeLoadCommands(function_ref<bool(const LoadCommand &)> ToRemove);
 
   void updateLoadCommandIndexes();
 
   /// Creates a new segment load command in the object and returns a reference
   /// to the newly created load command. The caller should verify that SegName
   /// is not too long (SegName.size() should be less than or equal to 16).
   LoadCommand &addSegment(StringRef SegName);
 
   bool is64Bit() const {
     return Header.Magic == MachO::MH_MAGIC_64 ||
            Header.Magic == MachO::MH_CIGAM_64;
   }
 
   uint64_t nextAvailableSegmentAddress() const;
 };
 
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm
 
 #endif // LLVM_OBJCOPY_MACHO_OBJECT_H

llvm/include/llvm/ObjCopy/wasm/Object.h

 //===- Object.h -------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TOOLS_LLVM_OBJCOPY_WASM_OBJECT_H
-#define LLVM_TOOLS_LLVM_OBJCOPY_WASM_OBJECT_H
+#ifndef LLVM_LLVM_OBJCOPY_WASM_OBJECT_H
+#define LLVM_LLVM_OBJCOPY_WASM_OBJECT_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Object/Wasm.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include <vector>
 
 namespace llvm {

 private:
   std::vector<std::unique_ptr<MemoryBuffer>> OwnedContents;
 };
 
 } // end namespace wasm
 } // end namespace objcopy
 } // end namespace llvm
 
-#endif // LLVM_TOOLS_LLVM_OBJCOPY_WASM_OBJECT_H
+#endif // LLVM_LLVM_OBJCOPY_WASM_OBJECT_H

llvm/include/llvm/ObjCopy/wasm/Reader.h

 //===- Reader.h -------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TOOLS_LLVM_OBJCOPY_WASM_READER_H
-#define LLVM_TOOLS_LLVM_OBJCOPY_WASM_READER_H
+#ifndef LLVM_LLVM_OBJCOPY_WASM_READER_H
+#define LLVM_LLVM_OBJCOPY_WASM_READER_H
 
 #include "Object.h"
 
 namespace llvm {
 namespace objcopy {
 namespace wasm {
 
 class Reader {
 public:
   explicit Reader(const object::WasmObjectFile &O) : WasmObj(O) {}
   Expected<std::unique_ptr<Object>> create() const;
 
 private:
   const object::WasmObjectFile &WasmObj;
 };
 
 } // end namespace wasm
 } // end namespace objcopy
 } // end namespace llvm
 
-#endif // LLVM_TOOLS_LLVM_OBJCOPY_WASM_READER_H
+#endif // LLVM_LLVM_OBJCOPY_WASM_READER_H

llvm/include/llvm/ObjCopy/wasm/WasmObjcopy.h

 //===- WasmObjcopy.h -------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TOOLS_LLVM_OBJCOPY_WASM_WASMOBJCOPY_H
-#define LLVM_TOOLS_LLVM_OBJCOPY_WASM_WASMOBJCOPY_H
+#ifndef LLVM_OBJCOPY_WASM_WASMOBJCOPY_H
+#define LLVM_OBJCOPY_WASM_WASMOBJCOPY_H
 
 namespace llvm {
 class Error;
 
 namespace object {
 class WasmObjectFile;
 } // end namespace object
 
 namespace objcopy {
 struct CopyConfig;
 class Buffer;
 
 namespace wasm {
 Error executeObjcopyOnBinary(const CopyConfig &Config,
                              object::WasmObjectFile &In, Buffer &Out);
 
 } // end namespace wasm
 } // end namespace objcopy
 } // end namespace llvm
 
-#endif // LLVM_TOOLS_LLVM_OBJCOPY_WASM_WASMOBJCOPY_H
+#endif // LLVM_OBJCOPY_WASM_WASMOBJCOPY_H

llvm/include/llvm/ObjCopy/wasm/Writer.h

 //===- Writer.h -------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TOOLS_LLVM_OBJCOPY_WASM_WRITER_H
-#define LLVM_TOOLS_LLVM_OBJCOPY_WASM_WRITER_H
+#ifndef LLVM_OBJCOPY_WASM_WRITER_H
+#define LLVM_OBJCOPY_WASM_WRITER_H
 
-#include "Buffer.h"
-#include "Object.h"
+#include "llvm/ObjCopy/Buffer.h"
+#include "llvm/ObjCopy/wasm/Object.h"
 #include <cstdint>
 #include <vector>
 
 namespace llvm {
 namespace objcopy {
 namespace wasm {
 
 class Writer {
static SectionHeader createSectionHeader(const Section &S,
                                            size_t &SectionSize);
   size_t finalize();
 };
 
 } // end namespace wasm
 } // end namespace objcopy
 } // end namespace llvm
 
-#endif // LLVM_TOOLS_LLVM_OBJCOPY_WASM_WRITER_H
+#endif // LLVM_OBJCOPY_WASM_WRITER_H

llvm/lib/CMakeLists.txt

 add_subdirectory(Extensions)
 add_subdirectory(Frontend)
 add_subdirectory(Transforms)
 add_subdirectory(Linker)
 add_subdirectory(Analysis)
 add_subdirectory(LTO)
 add_subdirectory(MC)
 add_subdirectory(MCA)
 add_subdirectory(Object)
+add_subdirectory(ObjCopy)
 add_subdirectory(ObjectYAML)
 add_subdirectory(Option)
 add_subdirectory(Remarks)
 add_subdirectory(DebugInfo)
 add_subdirectory(ExecutionEngine)
 add_subdirectory(Target)
 add_subdirectory(AsmParser)
 add_subdirectory(LineEditor)

llvm/lib/LLVMBuild.txt

  Linker
  InterfaceStub
  IR
  IRReader
  LTO
  MC
  MCA
  Object
+ ObjCopy
  BinaryFormat
  ObjectYAML
  Option
  Remarks
  Passes
  ProfileData
  Support
  TableGen

llvm/lib/ObjCopy/Buffer.cpp

 //===- Buffer.cpp ---------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "Buffer.h"
+#include "llvm/ObjCopy/Buffer.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Process.h"
 #include <memory>
 
 namespace llvm {
 namespace objcopy {

llvm/lib/ObjCopy/CMakeLists.txt

+add_llvm_component_library(LLVMObjCopy
+  Buffer.cpp
+  CopyConfig.cpp
+  COFF/COFFObjcopy.cpp
+  COFF/Object.cpp
+  COFF/Reader.cpp
+  COFF/Writer.cpp
+  ELF/ELFConfig.cpp
+  ELF/ELFObjcopy.cpp
+  ELF/Object.cpp
+  MachO/MachOObjcopy.cpp
+  MachO/MachOReader.cpp
+  MachO/MachOWriter.cpp
+  MachO/MachOLayoutBuilder.cpp
+  MachO/Object.cpp
+  wasm/Object.cpp
+  wasm/Reader.cpp
+  wasm/Writer.cpp
+  wasm/WasmObjcopy.cpp
+
+  ADDITIONAL_HEADER_DIRS
+  ${LLVM_MAIN_INCLUDE_DIR}/llvm/ObjCopy
+
+  DEPENDS
+  intrinsics_gen
+  )

llvm/lib/ObjCopy/COFF/COFFObjcopy.cpp

 //===- COFFObjcopy.cpp ----------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "COFFObjcopy.h"
-#include "Buffer.h"
-#include "CopyConfig.h"
-#include "Object.h"
-#include "Reader.h"
-#include "Writer.h"
+#include "llvm/ObjCopy/COFF/COFFObjcopy.h"
+#include "llvm/ObjCopy/Buffer.h"
+#include "llvm/ObjCopy/COFF/Object.h"
+#include "llvm/ObjCopy/COFF/Reader.h"
+#include "llvm/ObjCopy/COFF/Writer.h"
+#include "llvm/ObjCopy/CopyConfig.h"
 
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Support/CRC.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Path.h"
 #include <cassert>
 

llvm/lib/ObjCopy/COFF/Object.cpp

 //===- Object.cpp ---------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "Object.h"
+#include "llvm/ObjCopy/COFF/Object.h"
 #include "llvm/ADT/DenseSet.h"
 #include <algorithm>
 
 namespace llvm {
 namespace objcopy {
 namespace coff {
 
 using namespace object;

llvm/lib/ObjCopy/COFF/Reader.cpp

 //===- Reader.cpp ---------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "Reader.h"
-#include "Object.h"
+#include "llvm/ObjCopy/COFF/Reader.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/COFF.h"
+#include "llvm/ObjCopy/COFF/Object.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cstddef>
 #include <cstdint>
 
 namespace llvm {
 namespace objcopy {
 namespace coff {

llvm/lib/ObjCopy/COFF/Writer.cpp

 //===- Writer.cpp ---------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "Writer.h"
-#include "Object.h"
+#include "llvm/ObjCopy/COFF/Writer.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/COFF.h"
+#include "llvm/ObjCopy/COFF/Object.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cstddef>
 #include <cstdint>
 
 namespace llvm {
 namespace objcopy {
 namespace coff {

llvm/lib/ObjCopy/CopyConfig.cpp

+//===- CopyConfig.cpp -----------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ObjCopy/CopyConfig.h"
+
+namespace llvm {
+namespace objcopy {
+
+Expected<NameOrPattern>
+NameOrPattern::create(StringRef Pattern, MatchStyle MS,
+                      llvm::function_ref<Error(Error)> ErrorCallback) {
+  switch (MS) {
+  case MatchStyle::Literal:
+    return NameOrPattern(Pattern);
+  case MatchStyle::Wildcard: {
+    SmallVector<char, 32> Data;
+    bool IsPositiveMatch = true;
+    if (Pattern[0] == '!') {
+      IsPositiveMatch = false;
+      Pattern = Pattern.drop_front();
+    }
+    Expected<GlobPattern> GlobOrErr = GlobPattern::create(Pattern);
+
+    // If we couldn't create it as a glob, report the error, but try again with
+    // a literal if the error reporting is non-fatal.
+    if (!GlobOrErr) {
+      if (Error E = ErrorCallback(GlobOrErr.takeError()))
+        return std::move(E);
+      return create(Pattern, MatchStyle::Literal, ErrorCallback);
+    }
+
+    return NameOrPattern(std::make_shared<GlobPattern>(*GlobOrErr),
+                         IsPositiveMatch);
+  }
+  case MatchStyle::Regex: {
+    SmallVector<char, 32> Data;
+    return NameOrPattern(std::make_shared<Regex>(
+        ("^" + Pattern.ltrim('^').rtrim('$') + "$").toStringRef(Data)));
+  }
+  }
+  llvm_unreachable("Unhandled llvm.objcopy.MatchStyle enum");
+}
+
+} // namespace objcopy
+} // namespace llvm

llvm/lib/ObjCopy/ELF/ELFConfig.cpp

 //===- ELFConfig.cpp ------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "CopyConfig.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/ObjCopy/CopyConfig.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 
 namespace llvm {
 namespace objcopy {
 namespace elf {
 
 static Expected<NewSymbolInfo> parseNewSymbolInfo(StringRef FlagValue,

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

 //===- ELFObjcopy.cpp -----------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "ELFObjcopy.h"
-#include "Buffer.h"
-#include "CopyConfig.h"
-#include "Object.h"
+#include "llvm/ObjCopy/ELF/ELFObjcopy.h"
 #include "llvm/ADT/BitmaskEnum.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/MC/MCTargetOptions.h"
+#include "llvm/ObjCopy/Buffer.h"
+#include "llvm/ObjCopy/CopyConfig.h"
+#include "llvm/ObjCopy/ELF/Object.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Object/ELFTypes.h"
 #include "llvm/Object/Error.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/Errc.h"
for (const auto &Phdr : *PhdrsOrErr) {
     Error Err = Error::success();
     for (auto Note : In.notes(Phdr, Err))
       if (Note.getType() == NT_GNU_BUILD_ID && Note.getName() == ELF_NOTE_GNU)
         return Note.getDesc();
     if (Err)
       return createFileError(Config.InputFilename, std::move(Err));
   }
 
-  return createFileError(
-      Config.InputFilename,
+  return createFileError(Config.InputFilename,
                          createStringError(llvm::errc::invalid_argument,
                                            "could not find build ID"));
 }
 
 static Expected<ArrayRef<uint8_t>>
 findBuildID(const CopyConfig &Config, const object::ELFObjectFileBase &In) {
   if (auto *O = dyn_cast<ELFObjectFile<ELF32LE>>(&In))
     return findBuildID(Config, *O->getELFFile());
   else if (auto *O = dyn_cast<ELFObjectFile<ELF64LE>>(&In))
     return findBuildID(Config, *O->getELFFile());
   else if (auto *O = dyn_cast<ELFObjectFile<ELF32BE>>(&In))
     return findBuildID(Config, *O->getELFFile());
   else if (auto *O = dyn_cast<ELFObjectFile<ELF64BE>>(&In))
     return findBuildID(Config, *O->getELFFile());
 
   llvm_unreachable("Bad file format");
 }
 
 template <class... Ts>
-static Error makeStringError(std::error_code EC, const Twine &Msg, Ts &&... Args) {
+static Error makeStringError(std::error_code EC, const Twine &Msg,
+                             Ts &&... Args) {
   std::string FullMsg = (EC.message() + ": " + Msg).str();
   return createStringError(EC, FullMsg.c_str(), std::forward<Ts>(Args)...);
 }
 
 #define MODEL_8 "%%%%%%%%"
 #define MODEL_16 MODEL_8 MODEL_8
 #define MODEL_32 (MODEL_16 MODEL_16)
 

 Error executeObjcopyOnIHex(const CopyConfig &Config, MemoryBuffer &In,
                            Buffer &Out) {
   IHexReader Reader(&In);
   Expected<std::unique_ptr<Object>> Obj = Reader.create(true);
   if (!Obj)
     return Obj.takeError();
 
   const ElfType OutputElfType =
       getOutputElfType(Config.OutputArch.getValueOr(MachineInfo()));
   if (Error E = handleArgs(Config, **Obj, Reader, OutputElfType))
     return E;
   return writeOutput(Config, **Obj, Out, OutputElfType);
 }
 
 Error executeObjcopyOnRawBinary(const CopyConfig &Config, MemoryBuffer &In,
                                 Buffer &Out) {
   uint8_t NewSymbolVisibility =

llvm/lib/ObjCopy/ELF/Object.cpp

 //===- Object.cpp ---------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "Object.h"
+#include "llvm/ObjCopy/ELF/Object.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/MC/MCTargetOptions.h"
 #include "llvm/Object/ELF.h"
template <class ELFT> void ELFWriter<ELFT>::writePhdr(const Segment &Seg) {
   Phdr.p_vaddr = Seg.VAddr;
   Phdr.p_paddr = Seg.PAddr;
   Phdr.p_filesz = Seg.FileSize;
   Phdr.p_memsz = Seg.MemSize;
   Phdr.p_align = Seg.Align;
 }
 
 Error SectionBase::removeSectionReferences(
-    bool AllowBrokenLinks,
-    function_ref<bool(const SectionBase *)> ToRemove) {
+    bool AllowBrokenLinks, function_ref<bool(const SectionBase *)> ToRemove) {
   return Error::success();
 }
 
 Error SectionBase::removeSymbols(function_ref<bool(const Symbol &)> ToRemove) {
   return Error::success();
 }
 
 Error SectionBase::initialize(SectionTableRef) { return Error::success(); }
void SymbolTableSection::addSymbol(Twine Name, uint8_t Bind, uint8_t Type,
   Sym.Visibility = Visibility;
   Sym.Size = SymbolSize;
   Sym.Index = Symbols.size();
   Symbols.emplace_back(std::make_unique<Symbol>(Sym));
   Size += this->EntrySize;
 }
 
 Error SymbolTableSection::removeSectionReferences(
-    bool AllowBrokenLinks,
-    function_ref<bool(const SectionBase *)> ToRemove) {
+    bool AllowBrokenLinks, function_ref<bool(const SectionBase *)> ToRemove) {
   if (ToRemove(SectionIndexTable))
     SectionIndexTable = nullptr;
   if (ToRemove(SymbolNames)) {
     if (!AllowBrokenLinks)
       return createStringError(
           llvm::errc::invalid_argument,
           "string table '%s' cannot be removed because it is "
           "referenced by the symbol table '%s'",
Error SymbolTableSection::accept(SectionVisitor &Visitor) const {
   return Visitor.visit(*this);
 }
 
 Error SymbolTableSection::accept(MutableSectionVisitor &Visitor) {
   return Visitor.visit(*this);
 }
 
 Error RelocationSection::removeSectionReferences(
-    bool AllowBrokenLinks,
-    function_ref<bool(const SectionBase *)> ToRemove) {
+    bool AllowBrokenLinks, function_ref<bool(const SectionBase *)> ToRemove) {
   if (ToRemove(Symbols)) {
     if (!AllowBrokenLinks)
       return createStringError(
           llvm::errc::invalid_argument,
           "symbol table '%s' cannot be removed because it is "
           "referenced by the relocation section '%s'",
           Symbols->Name.data(), this->Name.data());
     Symbols = nullptr;
auto Data = ArrayRef<uint8_t>(
       MemBuf->getBufferSize());
   auto &DataSection = Obj->addSection<Section>(Data);
   DataSection.Name = ".data";
   DataSection.Type = ELF::SHT_PROGBITS;
   DataSection.Size = Data.size();
   DataSection.Flags = ELF::SHF_ALLOC | ELF::SHF_WRITE;
 
   std::string SanitizedFilename = MemBuf->getBufferIdentifier().str();
   std::replace_if(std::begin(SanitizedFilename), std::end(SanitizedFilename),
                   [](char C) { return !isalnum(C); }, '_');
   Twine Prefix = Twine("_binary_") + SanitizedFilename;
 
   SymTab->addSymbol(Prefix + "_start", STB_GLOBAL, STT_NOTYPE, &DataSection,
                     /*Value=*/0, NewSymbolVisibility, 0, 0);
   SymTab->addSymbol(Prefix + "_end", STB_GLOBAL, STT_NOTYPE, &DataSection,
                     /*Value=*/DataSection.Size, NewSymbolVisibility, 0, 0);
   SymTab->addSymbol(Prefix + "_size", STB_GLOBAL, STT_NOTYPE, nullptr,

 }
 
 template <class ELFT>
 ELFWriter<ELFT>::ELFWriter(Object &Obj, Buffer &Buf, bool WSH,
                            bool OnlyKeepDebug)
     : Writer(Obj, Buf), WriteSectionHeaders(WSH && Obj.HadShdrs),
       OnlyKeepDebug(OnlyKeepDebug) {}
 
-Error Object::removeSections(bool AllowBrokenLinks,
-    std::function<bool(const SectionBase &)> ToRemove) {
+Error Object::removeSections(
+    bool AllowBrokenLinks, std::function<bool(const SectionBase &)> ToRemove) {
 
   auto Iter = std::stable_partition(
       std::begin(Sections), std::end(Sections), [=](const SecPtr &Sec) {
         if (ToRemove(*Sec))
           return false;
         if (auto RelSec = dyn_cast<RelocationSectionBase>(Sec.get())) {
           if (auto ToRelSec = RelSec->getSection())
             return !ToRemove(*ToRelSec);
Error Object::removeSections(
   }
 
   // For each section that remains alive, we want to remove the dead references.
   // This either might update the content of the section (e.g. remove symbols
   // from symbol table that belongs to removed section) or trigger an error if
   // a live section critically depends on a section being removed somehow
   // (e.g. the removed section is referenced by a relocation).
   for (auto &KeepSec : make_range(std::begin(Sections), Iter)) {
-    if (Error E = KeepSec->removeSectionReferences(AllowBrokenLinks,
-            [&RemoveSections](const SectionBase *Sec) {
+    if (Error E = KeepSec->removeSectionReferences(
+            AllowBrokenLinks, [&RemoveSections](const SectionBase *Sec) {
               return RemoveSections.find(Sec) != RemoveSections.end();
             }))
       return E;
   }
 
   // Transfer removed sections into the Object RemovedSections container for use
   // later.
   std::move(Iter, Sections.end(), std::back_inserter(RemovedSections));
Error IHexWriter::write() {
   return Buf.commit();
 }
 
 Error IHexWriter::checkSection(const SectionBase &Sec) {
   uint64_t Addr = sectionPhysicalAddr(&Sec);
   if (addressOverflows32bit(Addr) || addressOverflows32bit(Addr + Sec.Size - 1))
     return createStringError(
         errc::invalid_argument,
-        "Section '%s' address range [0x%llx, 0x%llx] is not 32 bit", Sec.Name.c_str(),
-        Addr, Addr + Sec.Size - 1);
+        "Section '%s' address range [0x%llx, 0x%llx] is not 32 bit",
+        Sec.Name.c_str(), Addr, Addr + Sec.Size - 1);
   return Error::success();
 }
 
 Error IHexWriter::finalize() {
   bool UseSegments = false;
   auto ShouldWrite = [](const SectionBase &Sec) {
     return (Sec.Flags & ELF::SHF_ALLOC) && (Sec.Type != ELF::SHT_NOBITS);
   };

llvm/lib/ObjCopy/LLVMBuild.txt

-;===- ./lib/LLVMBuild.txt --------------------------------------*- Conf -*--===;
+;===- ./lib/ObjCopy/LLVMBuild.txt -----------------------------*- Conf -*--===;
 ;
 ; Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 ; See https://llvm.org/LICENSE.txt for license information.
 ; SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 ;
 ;===------------------------------------------------------------------------===;
 ;
 ; This is an LLVMBuild description file for the components in this subdirectory.
 ;
 ; For more information on the LLVMBuild system, please see:
 ;
 ;   http://llvm.org/docs/LLVMBuild.html
 ;
 ;===------------------------------------------------------------------------===;
 
-[common]
-subdirectories =
- Analysis
- AsmParser
- Bitcode
- Bitstream
- CodeGen
- DebugInfo
- Demangle
- DWARFLinker
- ExecutionEngine
- Extensions
- Frontend
- FuzzMutate
- LineEditor
- Linker
- InterfaceStub
- IR
- IRReader
- LTO
- MC
- MCA
- Object
- BinaryFormat
- ObjectYAML
- Option
- Remarks
- Passes
- ProfileData
- Support
- TableGen
- TextAPI
- Target
- Testing
- ToolDrivers
- Transforms
- WindowsManifest
- XRay
-
 [component_0]
-type = Group
-name = Libraries
-parent = $ROOT
+type = Library
+name = ObjCopy
+parent = Libraries
+required_libraries = Object Support MC

llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp

 //===- MachOLayoutBuilder.cpp -----------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "MachOLayoutBuilder.h"
+#include "llvm/ObjCopy/MachO/MachOLayoutBuilder.h"
 #include "llvm/Support/Alignment.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
 
 namespace llvm {
 namespace objcopy {
 namespace macho {
 

llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp

 //===- MachOObjcopy.cpp -----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "MachOObjcopy.h"
-#include "../CopyConfig.h"
-#include "MachOReader.h"
-#include "MachOWriter.h"
+#include "llvm/ObjCopy/MachO/MachOObjcopy.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ObjCopy/CopyConfig.h"
+#include "llvm/ObjCopy/MachO/MachOReader.h"
+#include "llvm/ObjCopy/MachO/MachOWriter.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 
 namespace llvm {
 namespace objcopy {
 namespace macho {
 
 using namespace object;

llvm/lib/ObjCopy/MachO/MachOReader.cpp

 //===- MachOReader.cpp ------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "MachOReader.h"
-#include "Object.h"
+#include "llvm/ObjCopy/MachO/MachOReader.h"
 #include "llvm/BinaryFormat/MachO.h"
+#include "llvm/ObjCopy/MachO/Object.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Support/Errc.h"
 #include <memory>
 
 namespace llvm {
 namespace objcopy {
 namespace macho {
 

llvm/lib/ObjCopy/MachO/MachOWriter.cpp

 //===- MachOWriter.cpp ------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "MachOWriter.h"
-#include "MachOLayoutBuilder.h"
-#include "Object.h"
+#include "llvm/ObjCopy/MachO/MachOWriter.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/BinaryFormat/MachO.h"
+#include "llvm/ObjCopy/MachO/MachOLayoutBuilder.h"
+#include "llvm/ObjCopy/MachO/Object.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <memory>
 
 namespace llvm {
 namespace objcopy {
 namespace macho {

llvm/lib/ObjCopy/MachO/Object.cpp

 //===- Object.cpp - Mach-O object file model --------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "Object.h"
+#include "llvm/ObjCopy/MachO/Object.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include <unordered_set>
 
 namespace llvm {
 namespace objcopy {
 namespace macho {
 
 const SymbolEntry *SymbolTable::getSymbolByIndex(uint32_t Index) const {

llvm/lib/ObjCopy/wasm/Object.cpp

 //===- Object.cpp ---------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "Object.h"
+#include "llvm/ObjCopy/wasm/Object.h"
 
 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace llvm {
 namespace objcopy {
 namespace wasm {
 

llvm/lib/ObjCopy/wasm/Reader.cpp

 //===- Reader.cpp ---------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "Reader.h"
+#include "llvm/ObjCopy/wasm/Reader.h"
 
 namespace llvm {
 namespace objcopy {
 namespace wasm {
 
 using namespace object;
 using namespace llvm::wasm;
 

llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp

 //===- WasmObjcopy.cpp ----------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "WasmObjcopy.h"
-#include "Buffer.h"
-#include "CopyConfig.h"
-#include "Object.h"
-#include "Reader.h"
-#include "Writer.h"
+#include "llvm/ObjCopy/wasm/WasmObjcopy.h"
+#include "llvm/ObjCopy/Buffer.h"
+#include "llvm/ObjCopy/CopyConfig.h"
+#include "llvm/ObjCopy/wasm/Object.h"
+#include "llvm/ObjCopy/wasm/Reader.h"
+#include "llvm/ObjCopy/wasm/Writer.h"
 #include "llvm/Support/Errc.h"
 
 namespace llvm {
 namespace objcopy {
 namespace wasm {
 
 using namespace object;
 

llvm/lib/ObjCopy/wasm/Writer.cpp

 //===- Writer.cpp ---------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "Writer.h"
+#include "llvm/ObjCopy/wasm/Writer.h"
 #include "llvm/BinaryFormat/Wasm.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace llvm {
 namespace objcopy {
 namespace wasm {

llvm/tools/llvm-objcopy/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Object
+  ObjCopy
   Option
   Support
   MC
   )
 
 set(LLVM_TARGET_DEFINITIONS ObjcopyOpts.td)
 tablegen(LLVM ObjcopyOpts.inc -gen-opt-parser-defs)
 add_public_tablegen_target(ObjcopyOptsTableGen)
 
 set(LLVM_TARGET_DEFINITIONS InstallNameToolOpts.td)
 tablegen(LLVM InstallNameToolOpts.inc -gen-opt-parser-defs)
 add_public_tablegen_target(InstallNameToolOptsTableGen)
 
 set(LLVM_TARGET_DEFINITIONS BitcodeStripOpts.td)
 tablegen(LLVM BitcodeStripOpts.inc -gen-opt-parser-defs)
 add_public_tablegen_target(BitcodeStripOptsTableGen)
 
 set(LLVM_TARGET_DEFINITIONS StripOpts.td)
 tablegen(LLVM StripOpts.inc -gen-opt-parser-defs)
 add_public_tablegen_target(StripOptsTableGen)
 
 add_llvm_tool(llvm-objcopy
-  Buffer.cpp
-  CopyConfig.cpp
   llvm-objcopy.cpp
-  COFF/COFFObjcopy.cpp
-  COFF/Object.cpp
-  COFF/Reader.cpp
-  COFF/Writer.cpp
-  ELF/ELFConfig.cpp
-  ELF/ELFObjcopy.cpp
-  ELF/Object.cpp
-  MachO/MachOObjcopy.cpp
-  MachO/MachOReader.cpp
-  MachO/MachOWriter.cpp
-  MachO/MachOLayoutBuilder.cpp
-  MachO/Object.cpp
-  wasm/Object.cpp
-  wasm/Reader.cpp
-  wasm/Writer.cpp
-  wasm/WasmObjcopy.cpp
+  ParseConfig.cpp
   DEPENDS
   ObjcopyOptsTableGen
   InstallNameToolOptsTableGen
   StripOptsTableGen
   )
 
 add_llvm_tool_symlink(llvm-install-name-tool llvm-objcopy)
 add_llvm_tool_symlink(llvm-bitcode-strip llvm-objcopy)

llvm/tools/llvm-objcopy/ParseConfig.h

+//===- ParseConfig.h ------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TOOLS_LLVM_OBJCOPY_PARSE_CONFIG_H
+#define LLVM_TOOLS_LLVM_OBJCOPY_PARSE_CONFIG_H
+
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ObjCopy/CopyConfig.h"
+#include "llvm/Support/Allocator.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/GlobPattern.h"
+#include "llvm/Support/Regex.h"
+
+namespace llvm {
+namespace objcopy {
+
+// Configuration for the overall invocation of this tool. When invoked as
+// objcopy, will always contain exactly one CopyConfig. When invoked as strip,
+// will contain one or more CopyConfigs.
+struct DriverConfig {
+  SmallVector<CopyConfig, 1> CopyConfigs;
+  BumpPtrAllocator Alloc;
+};
+
+// ParseObjcopyOptions returns the config and sets the input arguments. If a
+// help flag is set then ParseObjcopyOptions will print the help messege and
+// exit. ErrorCallback is used to handle recoverable errors. An Error returned
+// by the callback aborts the parsing and is then returned by this function.
+Expected<DriverConfig>
+parseObjcopyOptions(ArrayRef<const char *> ArgsArr,
+                    llvm::function_ref<Error(Error)> ErrorCallback);
+
+// ParseInstallNameToolOptions returns the config and sets the input arguments.
+// If a help flag is set then ParseInstallNameToolOptions will print the help
+// messege and exit.
+Expected<DriverConfig>
+parseInstallNameToolOptions(ArrayRef<const char *> ArgsArr);
+
+// ParseBitcodeStripOptions returns the config and sets the input arguments.
+// If a help flag is set then ParseBitcodeStripOptions will print the help
+// messege and exit.
+Expected<DriverConfig> parseBitcodeStripOptions(ArrayRef<const char *> ArgsArr);
+
+// ParseStripOptions returns the config and sets the input arguments. If a
+// help flag is set then ParseStripOptions will print the help messege and
+// exit. ErrorCallback is used to handle recoverable errors. An Error returned
+// by the callback aborts the parsing and is then returned by this function.
+Expected<DriverConfig>
+parseStripOptions(ArrayRef<const char *> ArgsArr,
+                  llvm::function_ref<Error(Error)> ErrorCallback);
+
+} // namespace objcopy
+} // namespace llvm
+
+#endif // LLVM_TOOLS_LLVM_OBJCOPY_PARSE_CONFIG_H

llvm/tools/llvm-objcopy/ParseConfig.cpp

-//===- CopyConfig.cpp -----------------------------------------------------===//
+//===- ParseConfig.cpp ----------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "CopyConfig.h"
-
+#include "ParseConfig.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CRC.h"
 #include "llvm/Support/CommandLine.h"
return createStringError(
         "bad format for --set-section-alignment: missing '='");
   auto Split = StringRef(FlagValue).split('=');
   if (Split.first.empty())
     return createStringError(
         errc::invalid_argument,
         "bad format for --set-section-alignment: missing section name");
   uint64_t NewAlign;
   if (Split.second.getAsInteger(0, NewAlign))
-    return createStringError(errc::invalid_argument,
-                             "invalid alignment for --set-section-alignment: '%s'",
-                             Split.second.str().c_str());
+    return createStringError(
+        errc::invalid_argument,
   return std::make_pair(Split.first, NewAlign);
 }
 
 static Expected<SectionFlagsUpdate>
 parseSetSectionFlagValue(StringRef FlagValue) {
   if (!StringRef(FlagValue).contains('='))
     return createStringError(errc::invalid_argument,
                              "bad format for --set-section-flags: missing '='");
if (!TrimmedLine.empty())
       if (Error E = Symbols.addMatcher(NameOrPattern::create(
               Saver.save(TrimmedLine), MS, ErrorCallback)))
         return E;
   }
 
   return Error::success();
 }
 
-Expected<NameOrPattern>
-NameOrPattern::create(StringRef Pattern, MatchStyle MS,
-                      llvm::function_ref<Error(Error)> ErrorCallback) {
-  switch (MS) {
-  case MatchStyle::Literal:
-    return NameOrPattern(Pattern);
-  case MatchStyle::Wildcard: {
-    SmallVector<char, 32> Data;
-    bool IsPositiveMatch = true;
-    if (Pattern[0] == '!') {
-      IsPositiveMatch = false;
-      Pattern = Pattern.drop_front();
-    }
-    Expected<GlobPattern> GlobOrErr = GlobPattern::create(Pattern);
-
-    // If we couldn't create it as a glob, report the error, but try again with
-    // a literal if the error reporting is non-fatal.
-    if (!GlobOrErr) {
-      if (Error E = ErrorCallback(GlobOrErr.takeError()))
-        return std::move(E);
-      return create(Pattern, MatchStyle::Literal, ErrorCallback);
-    }
-
-    return NameOrPattern(std::make_shared<GlobPattern>(*GlobOrErr),
-                         IsPositiveMatch);
-  }
-  case MatchStyle::Regex: {
-    SmallVector<char, 32> Data;
-    return NameOrPattern(std::make_shared<Regex>(
-        ("^" + Pattern.ltrim('^').rtrim('$') + "$").toStringRef(Data)));
-  }
-  }
-  llvm_unreachable("Unhandled llvm.objcopy.MatchStyle enum");
-}
-
 static Error addSymbolsToRenameFromFile(StringMap<StringRef> &SymbolsToRename,
                                         BumpPtrAllocator &Alloc,
                                         StringRef Filename) {
   StringSaver Saver(Alloc);
   SmallVector<StringRef, 16> Lines;
   auto BufOrErr = MemoryBuffer::getFile(Filename);
   if (!BufOrErr)
     return createFileError(Filename, BufOrErr.getError());

llvm/tools/llvm-objcopy/llvm-objcopy.cpp

 //===- llvm-objcopy.cpp ---------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "Buffer.h"
-#include "COFF/COFFObjcopy.h"
-#include "CopyConfig.h"
-#include "ELF/ELFObjcopy.h"
-#include "MachO/MachOObjcopy.h"
-#include "wasm/WasmObjcopy.h"
+#include "ParseConfig.h"
+#include "llvm/ObjCopy/Buffer.h"
+#include "llvm/ObjCopy/COFF/COFFObjcopy.h"
+#include "llvm/ObjCopy/CopyConfig.h"
+#include "llvm/ObjCopy/ELF/ELFObjcopy.h"
+#include "llvm/ObjCopy/MachO/MachOObjcopy.h"
+#include "llvm/ObjCopy/wasm/WasmObjcopy.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/ArchiveWriter.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/COFF.h"

llvm/unittests/CMakeLists.txt

 add_subdirectory(InterfaceStub)
 add_subdirectory(IR)
 add_subdirectory(LineEditor)
 add_subdirectory(Linker)
 add_subdirectory(MC)
 add_subdirectory(MI)
 add_subdirectory(Object)
 add_subdirectory(ObjectYAML)
+add_subdirectory(ObjCopy)
 add_subdirectory(Option)
 add_subdirectory(Remarks)
 add_subdirectory(Passes)
 add_subdirectory(ProfileData)
 add_subdirectory(Support)
 add_subdirectory(TableGen)
 add_subdirectory(Target)
 add_subdirectory(TextAPI)
 add_subdirectory(Transforms)
 add_subdirectory(XRay)
 add_subdirectory(tools)

llvm/unittests/ObjCopy/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS
+  Object
+  ObjCopy
+  ObjectYAML
+  )
+
+add_llvm_unittest(ObjCopyTests
+  ObjCopyTest.cpp
+  )
+
+target_link_libraries(ObjCopyTests PRIVATE LLVMTestingSupport)

llvm/unittests/ObjCopy/ObjCopyTest.cpp

+//===- ObjCopyTest.cpp ----------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ObjCopy/Buffer.h"
+#include "llvm/ObjCopy/COFF/COFFObjcopy.h"
+#include "llvm/ObjCopy/CopyConfig.h"
+#include "llvm/ObjCopy/ELF/ELFObjcopy.h"
+#include "llvm/ObjCopy/MachO/MachOObjcopy.h"
+#include "llvm/ObjCopy/wasm/WasmObjcopy.h"
+#include "llvm/Object/COFF.h"
+#include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Object/MachO.h"
+#include "llvm/Object/ObjectFile.h"
+#include "llvm/Object/Wasm.h"
+#include "llvm/ObjectYAML/yaml2obj.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/SmallVectorMemoryBuffer.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace object;
+using namespace objcopy;
+using namespace yaml;
+
+template <class T>
+void CopySimpleFileImpl(
+    const char *YamlCreationString,
+    std::function<Error(const CopyConfig &Config, T &In, Buffer &Out)>
+        ExecuteObjCopy,
+    std::function<bool(const Binary &File)> IsValidFormat) {
+  bool ErrorReported = false;
+  auto ErrHandler = [&](const Twine &) { ErrorReported = true; };
+
+  // Create Object file from yaml description.
+  SmallString<0> Storage;
+  std::unique_ptr<ObjectFile> Obj =
+      yaml2ObjectFile(Storage, YamlCreationString, ErrHandler);
+  ASSERT_FALSE(ErrorReported);
+  ASSERT_TRUE(Obj);
+  ASSERT_TRUE(IsValidFormat(*Obj));
+
+  T *TypedFile = dyn_cast<T>(Obj.get());
+  ASSERT_TRUE(TypedFile);
+
+  CopyConfig Config;
+  Config.OutputFilename = "a.out";
+  if (Obj->isELF()) {
+    Error ParseErr = Config.parseELFConfig();
+    ASSERT_FALSE(std::move(ParseErr));
+  }
+
+  // Call executeObjcopyOnBinary() from ObjCopy library.
+  objcopy::MemBuffer FB(Config.OutputFilename);
+  Error Err = ExecuteObjCopy(Config, *TypedFile, FB);
+  ASSERT_FALSE(std::move(Err));
+
+  // Load and check copied file.
+  std::unique_ptr<WritableMemoryBuffer> WFB = FB.releaseMemoryBuffer();
+  Expected<std::unique_ptr<ObjectFile>> Result =
+      ObjectFile::createObjectFile(WFB->getMemBufferRef());
+  ASSERT_THAT_EXPECTED(Result, Succeeded());
+  ASSERT_TRUE(IsValidFormat(**Result));
+}
+
+TEST(CopySimpleFile, COFF) {
+  CopySimpleFileImpl<object::COFFObjectFile>(
+      R"(
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:
+  - Name:            .text
+    Characteristics: [  ]
+    Alignment:       4
+    SectionData:     E800000000C3C3C3
+symbols:
+...
+)",
+      coff::executeObjcopyOnBinary,
+      [](const Binary &File) { return File.isCOFF(); });
+}
+
+TEST(CopySimpleFile, ELF) {
+  CopySimpleFileImpl<object::ELFObjectFileBase>(
+      R"(
+--- !ELF
+FileHeader:
+   Class:    ELFCLASS64
+   Data:     ELFDATA2LSB
+   Type:     ET_REL
+   Machine:  EM_X86_64)",
+      elf::executeObjcopyOnBinary,
+      [](const Binary &File) { return File.isELF(); });
+}
+
+TEST(CopySimpleFile, MachO) {
+  CopySimpleFileImpl<object::MachOObjectFile>(
+      R"(
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x01000007
+  cpusubtype:      0x80000003
+  filetype:        0x00000002
+  ncmds:           0
+  sizeofcmds:      0
+  flags:           0x00218085
+  reserved:        0x00000000
+...
+)",
+      macho::executeObjcopyOnBinary,
+      [](const Binary &File) { return File.isMachO(); });
+}
+
+TEST(CopySimpleFile, Wasm) {
+  CopySimpleFileImpl<object::WasmObjectFile>(
+      R"(
+--- !WASM
+FileHeader:
+  Version:         0x00000001
+...
+# CHECK: --- !WASM
+# CHECK: FileHeader:
+# CHECK:   Version:           0x00000001
+# CHECK: ...
+)",
+      objcopy::wasm::executeObjcopyOnBinary,
+      [](const Binary &File) { return File.isWasm(); });
+}