[llvm-objcopy] Add support for Intel HEX output format

[evgeny777]

Summary

We use IHEX to load ADSP firmware.

For implementation I used specification here:
http://www.interlog.com/~speff/usefulinfo/Hexfrmt.pdf

Also wiki article may seem useful
https://en.wikipedia.org/wiki/Intel_HEX

[rupprecht]

Thanks for sending this! I assigned PR39841 to you since this should fix that bug.

I ran this on some internal ihex files we have, and it seems to be dropping one of the sections, so I'll have to poke around to see what the bug is. But some ihex support is better than none, so it's not necessarily blocking :)

test/tools/llvm-objcopy/ELF/ihex-reader.test

2–4 ↗

(On Diff #193725)

The only ihex reading test (besides the error cases) is just consuming ihex that llvm-objcopy produces with -O ihex. Could you add a .hex test file for a more stable test? And then it would be easier to verify, e.g., llvm-objcopy -I ihex -O binary <test> is identical to objcopy -I ihex -O binary <test>

tools/llvm-objcopy/ELF/ELFObjcopy.cpp

638 ↗

(On Diff #193725)

A more succinct way (and same in executeObjcopyOnRawBinary):

const ElfType OutputElfType = getOutputElfType(
    Config.OutputArch.getValueOr(Config.BinaryArch));

tools/llvm-objcopy/ELF/Object.cpp

159–160 ↗

(On Diff #193725)

Fail is unused in release builds, so you need to add a (void)Fail; to silence the error/warning in release builds.

1050 ↗

(On Diff #193725)

RecAddr should be defined in the loop, where it is used

1450–1460 ↗

(On Diff #193725)

WDYT about just using llvm::Regex here instead of this method? It may be easier to read code if it just attempts to match ":[0-9A-F]+". It would produce less precise error messages, though.

1451 ↗

(On Diff #193725)

I think this will crash (or be UB) on an empty line?

1469–1470 ↗

(On Diff #193725)

I think there should be validation (somewhere) that there are no more records after this

1510 ↗

(On Diff #193725)

as a tiny optimization, call Records.reserve(Lines.size()) once you know how many lines there are.

1525–1532 ↗

(On Diff #193725)

Once we've validated it, can we convert the whole hex string to separate ArrayRef<uint8/16_t> fields for each record, so we don't have to worry about it being valid everywhere (i.e. using checkedGetHex)?

1534–1537 ↗

(On Diff #193725)

How about creating a static method to convert a line into an Expected<IHexRecord>, so we can return an error if it's invalid instead of making the user call getChecksum/checkRecord?

tools/llvm-objcopy/ELF/Object.h

195–198 ↗

(On Diff #193725)

Why not just uint16_t fields?

[evgeny777]

tools/llvm-objcopy/ELF/Object.cpp

1450–1460 ↗

(On Diff #193725)

I think that precise error message is more important. It might be hard in some cases to identify wrong character, e.g: I instead of 1, O instead of 0, russian A instead of english A and so on.

1451 ↗

(On Diff #193725)

Line is checked for minimal valid length earlier in the code. Though, it makes sense to assert here on !Line.empty()

1469–1470 ↗

(On Diff #193725)

I think that EndOfFile record should unconditionally cancel further processing. . This allows moving EOF record within a file to temporarily prevent part of records from loading. This can be useful for testing. Also it seems GNU objcopy behaves this way.

1525–1532 ↗

(On Diff #193725)

It's possible, but I don't see straight way to do this w/o dynamic memory allocation. As we're checking string with checkChars we shouldn't really step on conversion error, unless something really weird happens.

[evgeny777]

Line parser moved to IHexRecord::parse
Better test case for reader
Addressed some of review comments

[evgeny777]

Ping

[jhenderson]

In D60270#1463807, @evgeny777 wrote:

Ping

FYI, most of the other llvm-objcopy developers are still away following Euro LLVM, so it may not be until next week that you get an comments from @rupprecht. I don't currently have time to read up on ihex, I'm afraid, but if you don't get any feedback next week, I'll try to look into it if I have time.

[evgeny777]

FYI, most of the other llvm-objcopy developers are still away following Euro LLVM

Ah, I see. Ok, there is no rush.

[rupprecht]

In D60270#1464005, @evgeny777 wrote:

FYI, most of the other llvm-objcopy developers are still away following Euro LLVM

Ah, I see. Ok, there is no rush.

Sorry, I should have mentioned earlier that I was going to be busy last week. (In advance: I'm here this week, but I'll be out next week).
Hopefully I'll get to this one today, or if not, then tomorrow.

btw, some people at euro llvm also requested srec supprt, which seems extremely similar to ihex -- so it might be good to think about how generic this handling can be, e.g. maybe most of it should just be a "record" parser which is shared with ihex and srec. I don't think premature specialization should be done to make it more general than it should be, but just don't do anything that would be hostile towards refactoring it :)

[evgeny777]

btw, some people at euro llvm also requested srec supprt, which seems extremely similar to ihex

For me it doesn't look extremely similar to IHEX, except both formats use hexadecimal byte representation.
There are no such things as segment and extended addresses in SREC and even checksum calculation is different.

I think if we implement SREC then part of section builder functionality from IHexELFBuilder::addDataSections can be moved to a common base class,
also it seems SREC would have similar record structure (Type, Address, Data).

Still I expect writer and parser to be completely separate.

[jakehehrlich]

There's a lot of code to review here. I'll keep reviewing it everyday but this is going to take a while to review. Any help on splitting this up and making into smaller chunks would be helpful. Splitting reading and writing up into two separate patches would be helpful and removing features that we can add later would be helpful.

include/llvm/Support/Error.h

1180 ↗

(On Diff #193910)

size_t here and below is kind of confusing, can we use uint32_t?

tools/llvm-objcopy/ELF/Object.cpp

157 ↗

(On Diff #193910)

Unless an check that generates an error always proceeds this I think its best to return an error in this case, not assert fail. It would be better to roll this into an Expected function in that case I think anyway.

197 ↗

(On Diff #193910)

Maybe a raw_ostream would be useful here. We've generally avoided them but this format seems to lend itself to streams where as my opinion was the opposite before. You wouldn't need utohexstr since those formatting options are already supplied by the library I believe.

209 ↗

(On Diff #193910)

This is a very generic name with no comment. In general your comments have been awesome. I'd like to have an idea what this function does without reading the contents.

298 ↗

(On Diff #193910)

Does this ever make sense if there is no segment?

310 ↗

(On Diff #193910)

Masking that like this seems redundant, in general the number of places we're converting from 64 to 32 in an unchecked way is really shocking. I'd feel a lot more comfortable if we encapsulated these checks more and made them more clear.

313 ↗

(On Diff #193910)

Maybe we could split support for extended records out into a sperate patch and error out here for now?

tools/llvm-objcopy/ELF/Object.h

266 ↗

(On Diff #193910)

What's the point in splitting this into two classes?

Also does inheriting from BinarySectionWriter make sense? The same visitors will need to be implemented I would suppose the offsets and everything would be very different.

296 ↗

(On Diff #193910)

In general I think it might be worth considering weather there is a need to use sections at all. Originally with the binary writer we only used program headers. It turns out that people did a lot of stuff in a really odd way with GNU objcopy when using -O binary that required that we use sections as the primary basis for output. I would imagine that ihex users would not be doing the same sorts of odd tricks and that you could write the output strait from program headers. This would simplify the implementation greatly I think and harden the implementation against all sorts of odd corner cases.

498–507 ↗

(On Diff #193910)

Why do we need this to output a new format?

932 ↗

(On Diff #193910)

We can probably split this into two changes to make things smaller, one for reading, and one for writing yeah?

[evgeny777]

There's a lot of code to review here.

I've responded to some of the comments, meanwhile I'm splitting patch into writer (will go first) and reader (will go next). Will update the review soon

tools/llvm-objcopy/ELF/Object.cpp

157 ↗

(On Diff #193910)

This function can't actually return error, because string has been previously validated (see checkChars for example). IMO, it's bad practice to implement runtime checks for one's own logical errors.

197 ↗

(On Diff #193910)

This function was optimized to not using any dynamic allocation (IHexLineData is actually SmallVector), because each line contains only 16 bytes of section data, so it's possible to have really huge number of lines. What are the benefits of using raw_ostream?

298 ↗

(On Diff #193910)

It's a helper function which returns section VA if there is no segment. Any suggestion for better name?

310 ↗

(On Diff #193910)

There is a checkSections method which check all sections to detect if any of them has 64-bit address. Bear in mind that implementation also supports sign extended 32-bit addresses, i.e 0xFFFFFFFF80000000 is a valid address, but 0x100000000 is not

313 ↗

(On Diff #193910)

I suggest splitting reader and writer. To me it looks like a more logical split compared to removal of certain record types.

tools/llvm-objcopy/ELF/Object.h

266 ↗

(On Diff #193910)

I inherited IHexSectionWriter from BinarySectionWriter in order to reuse visitors for RelocationSection, GnuDebugLinkSection, e.t.c which will never go to IHEX nor to binary output. Are you suggesting duplication?

296 ↗

(On Diff #193910)

AFAIU one can't do this with IHEX, because unlike binary IHEX is not a contiguous blob, e.g you can have a gap between sections which won't go to output file.

498–507 ↗

(On Diff #193910)

This function takes a portion of hexadecimal data from IHexRecord and appends it in binary form to internal vector of OwnedDataSection

932 ↗

(On Diff #193910)

I think so.

[evgeny777]

Splitted IHEX patch into reader and writer. Diff now contains the "writer" part.

[evgeny777]

Any comments on this?

[evgeny777]

Ping

[rupprecht]

Looked mostly at the test for now, going to take a pass over the code today.

test/tools/llvm-objcopy/ELF/Inputs/ihex-elf-segments.yaml

20 ↗

(On Diff #197265)

I think this should be .data1?

Command Output (stderr):
--
error: Unknown section referenced: '.data1' by program header.

(I think this is a recent validation added by rL359663)

test/tools/llvm-objcopy/ELF/ihex-writer.test

1 ↗

(On Diff #197265)

All these hex outputs have diffs when compared to what GNU objcopy produces... is that expected? I haven't yet debugged exactly why.

3 ↗

(On Diff #197265)

cat X | FileCheck should be replaced with FileCheck --input-file=X everywhere

20 ↗

(On Diff #197265)

When I run GNU objcopy on this test case, I get an error: address 0xffffffff80001000 out of range for Intel Hex file. Maybe we shouldn't be supporting it? Are we able to handle it correctly somehow even though GNU objcopy can't?

tools/llvm-objcopy/ELF/Object.cpp

155 ↗

(On Diff #197265)

Isn't relying on Addr + 0x80000000 to loop around UB? Could this just directly check Addr & 0xffffffff80000000 == 0xffffffff80000000 instead?

[rupprecht]

Some insight into the differences:

test/tools/llvm-objcopy/ELF/ihex-writer.test

10 ↗

(On Diff #197265)

It looks like the addresses in this file don't match up, but I don't have a specific suggestion yet

60 ↗

(On Diff #197265)

It looks like this should only be printed when the address is not zero:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=bfd/ihex.c;h=101e0a76155fc48f95312c08307739cf9c1ee5eb;hb=HEAD#l880

tools/llvm-objcopy/ELF/Object.cpp

205 ↗

(On Diff #197265)

It looks like ihex uses \r\n line endings 😦

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=bfd/ihex.c;h=101e0a76155fc48f95312c08307739cf9c1ee5eb;hb=HEAD#l752

It seems weird for me to request this, but I think we should write \r\n, as this seems like a strange detail that people might need when consuming these files. I don't actually have any examples of this, however.

[MaskRay]

test/tools/llvm-objcopy/ELF/ihex-writer.test

9 ↗

(On Diff #197265)

The two RUN lines can be written as:
llvm-objcopy -O ihex %t-segs - | FileCheck --check-prefix=SEGMENTS %s if the output %t2-segs.hex isn't used elsewhere.

[evgeny777]

• Fixed error in section name in one of the tests
• Removed cat | FileCheck from test case
• Zero start address is not longer written to IHEX
• Switched to windows line endings.

test/tools/llvm-objcopy/ELF/ihex-writer.test

1 ↗

(On Diff #197265)

After I stopped emitting '03' record for zero start address output from ihex-elf-sections.yaml is identical to one of GNU objcopy.
However if input ELF file contains segments situation is different - GNU objcopy seems to ignore segments completely and always uses section virtual address. This doesn't seem logical to me and also doesn't look consistent with the way we're currently generating binary output in llvm-objcopy.

20 ↗

(On Diff #197265)

Probably the problem is in the version of objcopy you're using. On my machine 2.30 fails, but 2.32.51.20190227 works fine

tools/llvm-objcopy/ELF/Object.cpp

155 ↗

(On Diff #197265)

As far as I know unsigned overflows (unlike signed) are not UB. Addr is uint64_t.

205 ↗

(On Diff #197265)

Yes, I've seen this also. Nothing is said in IHEX spec about the line endings. Wikipedia tells that:

Programs that create HEX records typically use line termination characters that conform to the conventions of their operating systems

Probably the easiest thing to do is to stick to GNU behavior. I'll update the patch

[rupprecht]

I ran a few internal tests and this produces identical ihex output for every file I checked! \ o /

test/tools/llvm-objcopy/ELF/ihex-writer.test

20 ↗

(On Diff #197265)

Yep, that was it, I'm no longer seeing it with GNU objcopy from trunk.

llvm/trunk/include/llvm/Support/Error.h

 
 /// This class wraps a filename and another Error.
 ///
 /// In some cases, an error needs to live along a 'source' name, in order to
 /// show more detailed information to the user.
 class FileError final : public ErrorInfo<FileError> {
 
   friend Error createFileError(const Twine &, Error);
+  friend Error createFileError(const Twine &, size_t, Error);
 
 public:
   void log(raw_ostream &OS) const override {
     assert(Err && !FileName.empty() && "Trying to log after takeError().");
     OS << "'" << FileName << "': ";
+    if (Line.hasValue())
+      OS << "line " << Line.getValue() << ": ";
     Err->log(OS);
   }
 
   Error takeError() { return Error(std::move(Err)); }
 
   std::error_code convertToErrorCode() const override;
 
   // Used by ErrorInfo::classID.
   static char ID;
 
 private:
-  FileError(const Twine &F, std::unique_ptr<ErrorInfoBase> E) {
+  FileError(const Twine &F, Optional<size_t> LineNum,
+            std::unique_ptr<ErrorInfoBase> E) {
     assert(E && "Cannot create FileError from Error success value.");
     assert(!F.isTriviallyEmpty() &&
            "The file name provided to FileError must not be empty.");
     FileName = F.str();
     Err = std::move(E);
+    Line = std::move(LineNum);
   }
 
-  static Error build(const Twine &F, Error E) {
-    return Error(std::unique_ptr<FileError>(new FileError(F, E.takePayload())));
+  static Error build(const Twine &F, Optional<size_t> Line, Error E) {
+    return Error(
+        std::unique_ptr<FileError>(new FileError(F, Line, E.takePayload())));
   }
 
   std::string FileName;
+  Optional<size_t> Line;
   std::unique_ptr<ErrorInfoBase> Err;
 };
 
 /// Concatenate a source file path and/or name with an Error. The resulting
 /// Error is unchecked.
 inline Error createFileError(const Twine &F, Error E) {
-  return FileError::build(F, std::move(E));
+  return FileError::build(F, Optional<size_t>(), std::move(E));
+}
+
+/// Concatenate a source file path and/or name with line number and an Error.
+/// The resulting Error is unchecked.
+inline Error createFileError(const Twine &F, size_t Line, Error E) {
+  return FileError::build(F, Optional<size_t>(Line), std::move(E));
 }
 
 /// Concatenate a source file path and/or name with a std::error_code 
 /// to form an Error object.
 inline Error createFileError(const Twine &F, std::error_code EC) {
   return createFileError(F, errorCodeToError(EC));
 }
 
+/// Concatenate a source file path and/or name with line number and
+/// std::error_code to form an Error object.
+inline Error createFileError(const Twine &F, size_t Line, std::error_code EC) {
+  return createFileError(F, Line, errorCodeToError(EC));
+}
+
 Error createFileError(const Twine &F, ErrorSuccess) = delete;
 
 /// Helper for check-and-exit error handling.
 ///
 /// For tool use only. NOT FOR USE IN LIBRARY CODE.
 ///
 class ExitOnError {
 public:

llvm/trunk/test/tools/llvm-objcopy/ELF/Inputs/ihex-elf-pt-null.yaml

+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x0
+    AddressAlign:    0x8
+    Content:         "0001020304"
+ProgramHeaders:
+  - Type: PT_NULL
+    Flags: [ PF_X, PF_R ]
+    VAddr: 0xF00000000
+    PAddr: 0x100000
+    Sections:
+      - Section: .text

llvm/trunk/test/tools/llvm-objcopy/ELF/Inputs/ihex-elf-sections.yaml

+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .text
+# This section contents exceeds default IHex line length of 16 bytes
+# so we expect two lines created for it.
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x0
+    AddressAlign:    0x8
+    Content:         "000102030405060708090A0B0C0D0E0F1011121314"
+  - Name:            .data
+# This section overlap 16-bit segment boundary, so we expect
+# additional 'SegmentAddr' record of type '02'
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Content:         "3031323334353637383940"
+    Address:         0xFFF8
+    AddressAlign:    0x8
+  - Name:            .data2
+# Previous section '.data' should have forced creation of
+# 'SegmentAddr'(02) record with segment address of 0x10000,
+# so this section should have address of 0x100.
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Content:         "40414243"
+    Address:         0x10100
+    AddressAlign:    0x8
+  - Name:            .data3
+# The last section not only overlaps segment boundary, but
+# also has linear address which doesn't fit 20 bits. The 
+# following records should be craeted:
+# 'SegmentAddr'(02) record with address 0x0
+# 'ExtendedAddr'(04) record with address 0x100000
+# 'Data'(00) record with 8 bytes of section data
+# 'SegmentAddr'(02) record with address 0x10000
+# 'Data'(00) record with remaining 3 bytes of data.
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Content:         "5051525354555657585960"
+    Address:         0x10FFF8
+    AddressAlign:    0x8
+  - Name:            .bss
+# NOBITS sections are not written to IHex
+    Type:            SHT_NOBITS
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x10100
+    Size:            0x1000
+    AddressAlign:    0x8
+  - Name:            .dummy
+# Non-allocatable sections are not written to IHex
+    Type:            SHT_PROGBITS
+    Flags:           [ ]
+    Address:         0x20FFF8
+    Size:            65536
+    AddressAlign:    0x8

llvm/trunk/test/tools/llvm-objcopy/ELF/Inputs/ihex-elf-sections2.yaml

+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .text
+# Zero length sections are not exported to IHex
+# 'SegmentAddr' and 'ExtendedAddr' records aren't
+# created either.
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x7FFFFFFF
+    AddressAlign:    0x8
+    Size:            0
+  - Name:            .text1
+# Section address is sign-extended 32-bit address
+# Data fits 32-bit range
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0xFFFFFFFF80001000
+    AddressAlign:    0x8
+    Content:         "0001020304"
+  - Name:            .text2
+# Part of section data is in 32-bit address range
+# and part isn't.  
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0xFFFFFFF8
+    AddressAlign:    0x8
+    Content:         "000102030405060708"
+  - Name:            .text3
+  # Entire secion is outside of 32-bit range
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]    
+    Address:         0xFFFFFFFF0
+    AddressAlign:    0x8
+    Content:         "0001020304"

llvm/trunk/test/tools/llvm-objcopy/ELF/Inputs/ihex-elf-segments.yaml

+# Here we use yaml from ihex-elf-sections.yaml, but add single load
+# segment containing all exported sections. In such case we should 
+# use physical address of a section intead of virtual address. Physical
+# addresses start from 0x100000, so we create two additional 'ExtenededAddr'
+# (03) record in the beginning of IHex file with that physical address
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+  Entry:           0x100000
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x0
+    AddressAlign:    0x8
+    Content:         "000102030405060708090A0B0C0D0E0F1011121314"
+  - Name:            .data1
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Content:         "3031323334353637383940"
+    Address:         0xFFF8
+    AddressAlign:    0x8
+  - Name:            .data2
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Content:         "40414243"
+    Address:         0x10100
+    AddressAlign:    0x8
+  - Name:            .data3
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Content:         "5051525354555657585960"
+    Address:         0x10FFF8
+    AddressAlign:    0x8
+  - Name:            .bss
+    Type:            SHT_NOBITS
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x10100
+    Size:            0x1000
+    AddressAlign:    0x8
+  - Name:            .dummy
+    Type:            SHT_PROGBITS
+    Flags:           [ ]
+    Address:         0x20FFF8
+    Size:            65536
+    AddressAlign:    0x8
+ProgramHeaders:
+  - Type: PT_LOAD
+    Flags: [ PF_X, PF_R ]
+    VAddr: 0xF00000000
+    PAddr: 0x100000
+    Sections:
+      - Section: .text
+      - Section: .data1
+      - Section: .data2
+      - Section: .data3
+      - Section: .bss

llvm/trunk/test/tools/llvm-objcopy/ELF/ihex-writer.test

+# RUN: yaml2obj %p/Inputs/ihex-elf-sections.yaml -o %t
+# RUN: llvm-objcopy -O ihex %t - | FileCheck %s
+
+# Check ihex output, when we have segments in ELF file
+# In such case only sections in PT_LOAD segments will
+# be exported and their physical addresses will be used
+# RUN: yaml2obj %p/Inputs/ihex-elf-segments.yaml -o %t-segs
+# RUN: llvm-objcopy -O ihex %t-segs - | FileCheck %s --check-prefix=SEGMENTS
+
+# Check that non-load segments are ignored:
+# RUN: yaml2obj %p/Inputs/ihex-elf-pt-null.yaml -o %t2-segs
+# RUN: llvm-objcopy -O ihex %t2-segs - | FileCheck %s --check-prefix=PT_NULL
+
+# Check that sign-extended 32-bit section addresses are processed
+# correctly
+# RUN: yaml2obj %p/Inputs/ihex-elf-sections2.yaml -o %t-sec2
+# RUN: llvm-objcopy -O ihex --only-section=.text1 %t-sec2 - | FileCheck %s --check-prefix=SIGN_EXTENDED
+
+# Check that section address range overlapping 32 bit range
+# triggers an error
+# RUN: not llvm-objcopy -O ihex --only-section=.text2 %t-sec2 %t-sec2-2.hex 2>&1 | FileCheck %s --check-prefix=BAD-ADDR
+# RUN: not llvm-objcopy -O ihex --only-section=.text3 %t-sec2 %t-sec2-3.hex 2>&1 | FileCheck %s --check-prefix=BAD-ADDR2
+
+# Check that zero length section is not written
+# RUN: llvm-objcopy -O ihex --only-section=.text %t-sec2 - | FileCheck %s --check-prefix=ZERO_SIZE_SEC
+
+# Check 80x86 start address record. It is created for start
+# addresses less than 0x100000
+# RUN: llvm-objcopy -O ihex --set-start=0xFFFF %t - | FileCheck %s --check-prefix=START1
+
+# Check i386 start address record (05). It is created for
+# start addresses which doesn't fit 20 bits
+# RUN: llvm-objcopy -O ihex --set-start=0x100000 %t - | FileCheck %s --check-prefix=START2
+
+# We allow sign extended 32 bit start addresses as well.
+# RUN: llvm-objcopy -O ihex --set-start=0xFFFFFFFF80001000 %t - | FileCheck %s --check-prefix=START3
+
+# Start address which exceeds 32 bit range triggers an error
+# RUN: not llvm-objcopy -O ihex --set-start=0xF00000000 %t %t6.hex 2>&1 | FileCheck %s --check-prefix=BAD-START
+
+# CHECK:      :10000000000102030405060708090A0B0C0D0E0F78
+# CHECK-NEXT: :05001000101112131491
+# CHECK-NEXT: :08FFF800303132333435363765
+# CHECK-NEXT: :020000021000EC
+# CHECK-NEXT: :030000003839404C
+# CHECK-NEXT: :0401000040414243F5
+# CHECK-NEXT: :020000020000FC
+# CHECK-NEXT: :020000040010EA
+# CHECK-NEXT: :08FFF800505152535455565765
+# CHECK-NEXT: :020000040011E9
+# CHECK-NEXT: :03000000585960EC
+# CHECK-NEXT: :00000001FF
+
+# SEGMENTS:       :020000040010EA
+# SEGMENTS-NEXT:  :10000000000102030405060708090A0B0C0D0E0F78
+# SEGMENTS-NEXT:  :05001000101112131491
+# SEGMENTS-NEXT:  :0B001800303132333435363738394090
+# SEGMENTS-NEXT:  :0400280040414243CE
+# SEGMENTS-NEXT:  :0B003000505152535455565758596018
+# SEGMENTS-NEXT:  :0400000500100000E7
+# SEGMENTS-NEXT:  :00000001FF
+
+# 'ExtendedAddr' (04) record shouldn't be created
+# PT_NULL-NOT: :02000004
+
+# SIGN_EXTENDED:      :0200000480007A
+# SIGN_EXTENDED-NEXT: :051000000001020304E1
+# SIGN_EXTENDED-NEXT: :00000001FF
+
+# BAD-ADDR: error: {{.*}}: Section '.text2' address range [0xfffffff8, 0x100000000] is not 32 bit
+# BAD-ADDR2: error: {{.*}}: Section '.text3' address range [0xffffffff0, 0xffffffff4] is not 32 bit
+
+# There shouldn't be 'ExtendedAddr' nor 'Data' records
+# ZERO_SIZE_SEC-NOT:  :02000004
+# ZERO_SIZE_SEC-NOT:  :00FFFF00
+# ZERO_SIZE_SEC:      :00000001FF
+
+# START1: :040000030000FFFFFB
+# START2: :0400000500100000E7
+# START3: :040000058000100067
+# BAD-START: error: {{.*}}: Entry point address 0xf00000000 overflows 32 bits

llvm/trunk/tools/llvm-objcopy/CopyConfig.cpp

if (BinaryArch.empty())
       return createStringError(
           errc::invalid_argument,
           "Specified binary input without specifiying an architecture");
     Expected<const MachineInfo &> MI = getMachineInfo(BinaryArch);
     if (!MI)
       return MI.takeError();
     Config.BinaryArch = *MI;
   }
-  if (!Config.OutputFormat.empty() && Config.OutputFormat != "binary") {
+  if (!Config.OutputFormat.empty() && Config.OutputFormat != "binary" &&
+      Config.OutputFormat != "ihex") {
     Expected<MachineInfo> MI = getOutputFormatMachineInfo(Config.OutputFormat);
     if (!MI)
       return MI.takeError();
     Config.OutputArch = *MI;
   }
 
   if (auto Arg = InputArgs.getLastArg(OBJCOPY_compress_debug_sections,
                                       OBJCOPY_compress_debug_sections_eq)) {

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

 static ElfType getOutputElfType(const MachineInfo &MI) {
   // Infer output ELF type from the binary arch specified
   if (MI.Is64Bit)
     return MI.IsLittleEndian ? ELFT_ELF64LE : ELFT_ELF64BE;
   else
     return MI.IsLittleEndian ? ELFT_ELF32LE : ELFT_ELF32BE;
 }
 
-static std::unique_ptr<Writer> createWriter(const CopyConfig &Config,
+static std::unique_ptr<Writer> createELFWriter(const CopyConfig &Config,
                                                Object &Obj, Buffer &Buf,
                                                ElfType OutputElfType) {
-  if (Config.OutputFormat == "binary") {
-    return llvm::make_unique<BinaryWriter>(Obj, Buf);
-  }
   // Depending on the initial ELFT and OutputFormat we need a different Writer.
   switch (OutputElfType) {
   case ELFT_ELF32LE:
     return llvm::make_unique<ELFWriter<ELF32LE>>(Obj, Buf,
                                                  !Config.StripSections);
   case ELFT_ELF64LE:
     return llvm::make_unique<ELFWriter<ELF64LE>>(Obj, Buf,
                                                  !Config.StripSections);
   case ELFT_ELF32BE:
     return llvm::make_unique<ELFWriter<ELF32BE>>(Obj, Buf,
                                                  !Config.StripSections);
   case ELFT_ELF64BE:
     return llvm::make_unique<ELFWriter<ELF64BE>>(Obj, Buf,
                                                  !Config.StripSections);
   }
   llvm_unreachable("Invalid output format");
 }
 
+static std::unique_ptr<Writer> createWriter(const CopyConfig &Config,
+                                            Object &Obj, Buffer &Buf,
+                                            ElfType OutputElfType) {
+  using Functor = std::function<std::unique_ptr<Writer>()>;
+  return StringSwitch<Functor>(Config.OutputFormat)
+      .Case("binary", [&] { return llvm::make_unique<BinaryWriter>(Obj, Buf); })
+      .Case("ihex", [&] { return llvm::make_unique<IHexWriter>(Obj, Buf); })
+      .Default(
+          [&] { return createELFWriter(Config, Obj, Buf, OutputElfType); })();
+}
+
 template <class ELFT>
 static Expected<ArrayRef<uint8_t>>
 findBuildID(const CopyConfig &Config, const object::ELFFile<ELFT> &In) {
   auto PhdrsOrErr = In.program_headers();
   if (auto Err = PhdrsOrErr.takeError())
     return createFileError(Config.InputFilename, std::move(Err));
 
   for (const auto &Phdr : *PhdrsOrErr) {
Obj.SymbolTable->addSymbol(
         Sec ? (uint16_t)SYMBOL_SIMPLE_INDEX : (uint16_t)SHN_ABS, 0);
   }
 
   if (Config.EntryExpr)
     Obj.Entry = Config.EntryExpr(Obj.Entry);
   return Error::success();
 }
 
+static Error writeOutput(const CopyConfig &Config, Object &Obj, Buffer &Out,
+                         ElfType OutputElfType) {
+  std::unique_ptr<Writer> Writer =
+      createWriter(Config, Obj, Out, OutputElfType);
+  if (Error E = Writer->finalize())
+    return E;
+  return Writer->write();
+}
+
 Error executeObjcopyOnRawBinary(const CopyConfig &Config, MemoryBuffer &In,
                                 Buffer &Out) {
   BinaryReader Reader(Config.BinaryArch, &In);
   std::unique_ptr<Object> Obj = Reader.create();
 
   // Prefer OutputArch (-O<format>) if set, otherwise fallback to BinaryArch
   // (-B<arch>).
-  const ElfType OutputElfType = getOutputElfType(
-      Config.OutputArch ? Config.OutputArch.getValue() : Config.BinaryArch);
+  const ElfType OutputElfType =
+      getOutputElfType(Config.OutputArch.getValueOr(Config.BinaryArch));
   if (Error E = handleArgs(Config, *Obj, Reader, OutputElfType))
     return E;
-  std::unique_ptr<Writer> Writer =
-      createWriter(Config, *Obj, Out, OutputElfType);
-  if (Error E = Writer->finalize())
-    return E;
-  return Writer->write();
+  return writeOutput(Config, *Obj, Out, OutputElfType);
 }
 
 Error executeObjcopyOnBinary(const CopyConfig &Config,
                              object::ELFObjectFileBase &In, Buffer &Out) {
   ELFReader Reader(&In);
   std::unique_ptr<Object> Obj = Reader.create();
   // Prefer OutputArch (-O<format>) if set, otherwise infer it from the input.
   const ElfType OutputElfType =
if (!Config.BuildIdLinkDir.empty() && Config.BuildIdLinkInput)
     if (Error E =
             linkToBuildIdDir(Config, Config.InputFilename,
                              Config.BuildIdLinkInput.getValue(), BuildIdBytes))
       return E;
 
   if (Error E = handleArgs(Config, *Obj, Reader, OutputElfType))
     return createFileError(Config.InputFilename, std::move(E));
 
-  std::unique_ptr<Writer> Writer =
-      createWriter(Config, *Obj, Out, OutputElfType);
-  if (Error E = Writer->finalize())
+  if (Error E = writeOutput(Config, *Obj, Out, OutputElfType))
     return createFileError(Config.InputFilename, std::move(E));
-  if (Error E = Writer->write())
-    return E;
   if (!Config.BuildIdLinkDir.empty() && Config.BuildIdLinkOutput)
     if (Error E =
             linkToBuildIdDir(Config, Config.OutputFilename,
                              Config.BuildIdLinkOutput.getValue(), BuildIdBytes))
       return createFileError(Config.OutputFilename, std::move(E));
 
   return Error::success();
 }
 
 } // end namespace elf
 } // end namespace objcopy
 } // end namespace llvm

llvm/trunk/tools/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/Object/ELFObjectFile.h"
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include <cstddef>
 #include <cstdint>
 #include <functional>
 #include <memory>
 #include <set>
 #include <vector>
 
public:
   void visit(GroupSection &Sec) override;
   void visit(SectionIndexSection &Sec) override;
   void visit(CompressedSection &Sec) override;
   void visit(DecompressedSection &Sec) override;
 };
 
 #define MAKE_SEC_WRITER_FRIEND                                                 \
   friend class SectionWriter;                                                  \
+  friend class IHexSectionWriterBase;                                          \
+  friend class IHexSectionWriter;                                              \
   template <class ELFT> friend class ELFSectionWriter;                         \
   template <class ELFT> friend class ELFSectionSizer;
 
 class BinarySectionWriter : public SectionWriter {
 public:
   virtual ~BinarySectionWriter() {}
 
   void visit(const SymbolTableSection &Sec) override;
   void visit(const RelocationSection &Sec) override;
   void visit(const GnuDebugLinkSection &Sec) override;
   void visit(const GroupSection &Sec) override;
   void visit(const SectionIndexSection &Sec) override;
   void visit(const CompressedSection &Sec) override;
   void visit(const DecompressedSection &Sec) override;
 
   explicit BinarySectionWriter(Buffer &Buf) : SectionWriter(Buf) {}
 };
 
+using IHexLineData = SmallVector<char, 64>;
+
+struct IHexRecord {
+  // Memory address of the record.
+  uint16_t Addr;
+  // Record type (see below).
+  uint16_t Type;
+  // Record data in hexadecimal form.
+  StringRef HexData;
+
+  // Helper method to get file length of the record
+  // including newline character
+  static size_t getLength(size_t DataSize) {
+    // :LLAAAATT[DD...DD]CC'
+    return DataSize * 2 + 11;
+  }
+
+  // Gets length of line in a file (getLength + CRLF).
+  static size_t getLineLength(size_t DataSize) {
+    return getLength(DataSize) + 2;
+  }
+
+  // Given type, address and data returns line which can
+  // be written to output file.
+  static IHexLineData getLine(uint8_t Type, uint16_t Addr,
+                              ArrayRef<uint8_t> Data);
+
+  // Calculates checksum of stringified record representation
+  // S must NOT contain leading ':' and trailing whitespace
+  // characters
+  static uint8_t getChecksum(StringRef S);
+
+  enum Type {
+    // Contains data and a 16-bit starting address for the data.
+    // The byte count specifies number of data bytes in the record.
+    Data = 0,
+    // Must occur exactly once per file in the last line of the file.
+    // The data field is empty (thus byte count is 00) and the address
+    // field is typically 0000.
+    EndOfFile = 1,
+    // The data field contains a 16-bit segment base address (thus byte
+    // count is always 02) compatible with 80x86 real mode addressing.
+    // The address field (typically 0000) is ignored. The segment address
+    // from the most recent 02 record is multiplied by 16 and added to each
+    // subsequent data record address to form the physical starting address
+    // for the data. This allows addressing up to one megabyte of address
+    // space.
+    SegmentAddr = 2,
+    // or 80x86 processors, specifies the initial content of the CS:IP
+    // registers. The address field is 0000, the byte count is always 04,
+    // the first two data bytes are the CS value, the latter two are the
+    // IP value.
+    StartAddr80x86 = 3,
+    // Allows for 32 bit addressing (up to 4GiB). The record's address field
+    // is ignored (typically 0000) and its byte count is always 02. The two
+    // data bytes (big endian) specify the upper 16 bits of the 32 bit
+    // absolute address for all subsequent type 00 records
+    ExtendedAddr = 4,
+    // The address field is 0000 (not used) and the byte count is always 04.
+    // The four data bytes represent a 32-bit address value. In the case of
+    // 80386 and higher CPUs, this address is loaded into the EIP register.
+    StartAddr = 5,
+    // We have no other valid types
+    InvalidType = 6
+  };
+};
+
+// Base class for IHexSectionWriter. This class implements writing algorithm,
+// but doesn't actually write records. It is used for output buffer size
+// calculation in IHexWriter::finalize.
+class IHexSectionWriterBase : public BinarySectionWriter {
+  // 20-bit segment address
+  uint32_t SegmentAddr = 0;
+  // Extended linear address
+  uint32_t BaseAddr = 0;
+
+  // Write segment address corresponding to 'Addr'
+  uint64_t writeSegmentAddr(uint64_t Addr);
+  // Write extended linear (base) address corresponding to 'Addr'
+  uint64_t writeBaseAddr(uint64_t Addr);
+
+protected:
+  // Offset in the output buffer
+  uint64_t Offset = 0;
+
+  void writeSection(const SectionBase *Sec, ArrayRef<uint8_t> Data);
+  virtual void writeData(uint8_t Type, uint16_t Addr, ArrayRef<uint8_t> Data);
+
+public:
+  explicit IHexSectionWriterBase(Buffer &Buf) : BinarySectionWriter(Buf) {}
+
+  uint64_t getBufferOffset() const { return Offset; }
+  void visit(const Section &Sec) final;
+  void visit(const OwnedDataSection &Sec) final;
+  void visit(const StringTableSection &Sec) override;
+  void visit(const DynamicRelocationSection &Sec) final;
+  using BinarySectionWriter::visit;
+};
+
+// Real IHEX section writer
+class IHexSectionWriter : public IHexSectionWriterBase {
+public:
+  IHexSectionWriter(Buffer &Buf) : IHexSectionWriterBase(Buf) {}
+
+  void writeData(uint8_t Type, uint16_t Addr, ArrayRef<uint8_t> Data) override;
+  void visit(const StringTableSection &Sec) override;
+};
+
 class Writer {
 protected:
   Object &Obj;
   Buffer &Buf;
 
 public:
   virtual ~Writer();
   virtual Error finalize() = 0;

 
 public:
   ~BinaryWriter() {}
   Error finalize() override;
   Error write() override;
   BinaryWriter(Object &Obj, Buffer &Buf) : Writer(Obj, Buf) {}
 };
 
+class IHexWriter : public Writer {
+  struct SectionCompare {
+    bool operator()(const SectionBase *Lhs, const SectionBase *Rhs) const;
+  };
+
+  std::set<const SectionBase *, SectionCompare> Sections;
+  size_t TotalSize;
+
+  Error checkSection(const SectionBase &Sec);
+  uint64_t writeEntryPointRecord(uint8_t *Buf);
+  uint64_t writeEndOfFileRecord(uint8_t *Buf);
+
+public:
+  ~IHexWriter() {}
+  Error finalize() override;
+  Error write() override;
+  IHexWriter(Object &Obj, Buffer &Buf) : Writer(Obj, Buf) {}
+};
+
 class SectionBase {
 public:
   std::string Name;
   Segment *ParentSegment = nullptr;
   uint64_t HeaderOffset;
   uint64_t OriginalOffset = std::numeric_limits<uint64_t>::max();
   uint32_t Index;
   bool HasSymbol = false;
public:
   OwnedDataSection(StringRef SecName, ArrayRef<uint8_t> Data)
       : Data(std::begin(Data), std::end(Data)) {
     Name = SecName.str();
     Type = ELF::SHT_PROGBITS;
     Size = Data.size();
     OriginalOffset = std::numeric_limits<uint64_t>::max();
   }
 
+  OwnedDataSection(const Twine &SecName, uint64_t SecAddr, uint64_t SecFlags,
+                   uint64_t SecOff) {
+    Name = SecName.str();
+    Type = ELF::SHT_PROGBITS;
+    Addr = SecAddr;
+    Flags = SecFlags;
+    OriginalOffset = SecOff;
+  }
+
+  void appendHexData(StringRef HexData);
   void accept(SectionVisitor &Sec) const override;
   void accept(MutableSectionVisitor &Visitor) override;
 };
 
 class CompressedSection : public SectionBase {
   MAKE_SEC_WRITER_FRIEND
 
   DebugCompressionType CompressionType;

llvm/trunk/tools/llvm-objcopy/ELF/Object.cpp

 #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/ELFObjectFile.h"
 #include "llvm/Support/Compression.h"
-#include "llvm/Support/Errc.h"
+#include "llvm/Support/Endian.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/Path.h"
 #include <algorithm>
 #include <cstddef>
 #include <cstdint>
 #include <iterator>
 #include <unordered_set>
void BinarySectionWriter::visit(const GroupSection &Sec) {
   error("cannot write '" + Sec.Name + "' out to binary");
 }
 
 void SectionWriter::visit(const Section &Sec) {
   if (Sec.Type != SHT_NOBITS)
     llvm::copy(Sec.Contents, Out.getBufferStart() + Sec.Offset);
 }
 
+static bool addressOverflows32bit(uint64_t Addr) {
+  // Sign extended 32 bit addresses (e.g 0xFFFFFFFF80000000) are ok
+  return Addr > UINT32_MAX && Addr + 0x80000000 > UINT32_MAX;
+}
+
+template <class T> static T checkedGetHex(StringRef S) {
+  T Value;
+  bool Fail = S.getAsInteger(16, Value);
+  assert(!Fail);
+  (void)Fail;
+  return Value;
+}
+
+// Fills exactly Len bytes of buffer with hexadecimal characters
+// representing value 'X'
+template <class T, class Iterator>
+static Iterator utohexstr(T X, Iterator It, size_t Len) {
+  // Fill range with '0'
+  std::fill(It, It + Len, '0');
+
+  for (long I = Len - 1; I >= 0; --I) {
+    unsigned char Mod = static_cast<unsigned char>(X) & 15;
+    *(It + I) = hexdigit(Mod, false);
+    X >>= 4;
+  }
+  assert(X == 0);
+  return It + Len;
+}
+
+uint8_t IHexRecord::getChecksum(StringRef S) {
+  assert((S.size() & 1) == 0);
+  uint8_t Checksum = 0;
+  while (!S.empty()) {
+    Checksum += checkedGetHex<uint8_t>(S.take_front(2));
+    S = S.drop_front(2);
+  }
+  return -Checksum;
+}
+
+IHexLineData IHexRecord::getLine(uint8_t Type, uint16_t Addr,
+                                 ArrayRef<uint8_t> Data) {
+  IHexLineData Line(getLineLength(Data.size()));
+  assert(Line.size());
+  auto Iter = Line.begin();
+  *Iter++ = ':';
+  Iter = utohexstr(Data.size(), Iter, 2);
+  Iter = utohexstr(Addr, Iter, 4);
+  Iter = utohexstr(Type, Iter, 2);
+  for (uint8_t X : Data)
+    Iter = utohexstr(X, Iter, 2);
+  StringRef S(Line.data() + 1, std::distance(Line.begin() + 1, Iter));
+  Iter = utohexstr(getChecksum(S), Iter, 2);
+  *Iter++ = '\r';
+  *Iter++ = '\n';
+  assert(Iter == Line.end());
+  return Line;
+}
+
+static uint64_t sectionPhysicalAddr(const SectionBase *Sec) {
+  Segment *Seg = Sec->ParentSegment;
+  if (Seg && Seg->Type != ELF::PT_LOAD)
+    Seg = nullptr;
+  return Seg ? Seg->PAddr + Sec->OriginalOffset - Seg->OriginalOffset
+             : Sec->Addr;
+}
+
+void IHexSectionWriterBase::writeSection(const SectionBase *Sec,
+                                         ArrayRef<uint8_t> Data) {
+  assert(Data.size() == Sec->Size);
+  const uint32_t ChunkSize = 16;
+  uint32_t Addr = sectionPhysicalAddr(Sec) & 0xFFFFFFFFU;
+  while (!Data.empty()) {
+    uint64_t DataSize = std::min<uint64_t>(Data.size(), ChunkSize);
+    if (Addr > SegmentAddr + BaseAddr + 0xFFFFU) {
+      if (Addr > 0xFFFFFU) {
+        // Write extended address record, zeroing segment address
+        // if needed.
+        if (SegmentAddr != 0)
+          SegmentAddr = writeSegmentAddr(0U);
+        BaseAddr = writeBaseAddr(Addr);
+      } else {
+        // We can still remain 16-bit
+        SegmentAddr = writeSegmentAddr(Addr);
+      }
+    }
+    uint64_t SegOffset = Addr - BaseAddr - SegmentAddr;
+    assert(SegOffset <= 0xFFFFU);
+    DataSize = std::min(DataSize, 0x10000U - SegOffset);
+    writeData(0, SegOffset, Data.take_front(DataSize));
+    Addr += DataSize;
+    Data = Data.drop_front(DataSize);
+  }
+}
+
+uint64_t IHexSectionWriterBase::writeSegmentAddr(uint64_t Addr) {
+  assert(Addr <= 0xFFFFFU);
+  uint8_t Data[] = {static_cast<uint8_t>((Addr & 0xF0000U) >> 12), 0};
+  writeData(2, 0, Data);
+  return Addr & 0xF0000U;
+}
+
+uint64_t IHexSectionWriterBase::writeBaseAddr(uint64_t Addr) {
+  assert(Addr <= 0xFFFFFFFFU);
+  uint64_t Base = Addr & 0xFFFF0000U;
+  uint8_t Data[] = {static_cast<uint8_t>(Base >> 24),
+                    static_cast<uint8_t>((Base >> 16) & 0xFF)};
+  writeData(4, 0, Data);
+  return Base;
+}
+
+void IHexSectionWriterBase::writeData(uint8_t Type, uint16_t Addr,
+                                      ArrayRef<uint8_t> Data) {
+  Offset += IHexRecord::getLineLength(Data.size());
+}
+
+void IHexSectionWriterBase::visit(const Section &Sec) {
+  writeSection(&Sec, Sec.Contents);
+}
+
+void IHexSectionWriterBase::visit(const OwnedDataSection &Sec) {
+  writeSection(&Sec, Sec.Data);
+}
+
+void IHexSectionWriterBase::visit(const StringTableSection &Sec) {
+  // Check that sizer has already done its work
+  assert(Sec.Size == Sec.StrTabBuilder.getSize());
+  // We are free to pass an invalid pointer to writeSection as long
+  // as we don't actually write any data. The real writer class has
+  // to override this method .
+  writeSection(&Sec, {nullptr, Sec.Size});
+}
+
+void IHexSectionWriterBase::visit(const DynamicRelocationSection &Sec) {
+  writeSection(&Sec, Sec.Contents);
+}
+
+void IHexSectionWriter::writeData(uint8_t Type, uint16_t Addr,
+                                  ArrayRef<uint8_t> Data) {
+  IHexLineData HexData = IHexRecord::getLine(Type, Addr, Data);
+  memcpy(Out.getBufferStart() + Offset, HexData.data(), HexData.size());
+  Offset += HexData.size();
+}
+
+void IHexSectionWriter::visit(const StringTableSection &Sec) {
+  assert(Sec.Size == Sec.StrTabBuilder.getSize());
+  std::vector<uint8_t> Data(Sec.Size);
+  Sec.StrTabBuilder.write(Data.data());
+  writeSection(&Sec, Data);
+}
+
 void Section::accept(SectionVisitor &Visitor) const { Visitor.visit(*this); }
 
 void Section::accept(MutableSectionVisitor &Visitor) { Visitor.visit(*this); }
 
 void SectionWriter::visit(const OwnedDataSection &Sec) {
   llvm::copy(Sec.Data, Out.getBufferStart() + Sec.Offset);
 }
 

 void OwnedDataSection::accept(SectionVisitor &Visitor) const {
   Visitor.visit(*this);
 }
 
 void OwnedDataSection::accept(MutableSectionVisitor &Visitor) {
   Visitor.visit(*this);
 }
 
+void OwnedDataSection::appendHexData(StringRef HexData) {
+  assert((HexData.size() & 1) == 0);
+  while (!HexData.empty()) {
+    Data.push_back(checkedGetHex<uint8_t>(HexData.take_front(2)));
+    HexData = HexData.drop_front(2);
+  }
+  Size = Data.size();
+}
+
 void BinarySectionWriter::visit(const CompressedSection &Sec) {
   error("cannot write compressed section '" + Sec.Name + "' ");
 }
 
 template <class ELFT>
 void ELFSectionWriter<ELFT>::visit(const CompressedSection &Sec) {
   uint8_t *Buf = Out.getBufferStart() + Sec.Offset;
   if (Sec.CompressionType == DebugCompressionType::None) {
if (Section->Type != SHT_NOBITS)
       TotalSize = std::max(TotalSize, Section->Offset + Section->Size);
 
   if (Error E = Buf.allocate(TotalSize))
     return E;
   SecWriter = llvm::make_unique<BinarySectionWriter>(Buf);
   return Error::success();
 }
 
+bool IHexWriter::SectionCompare::operator()(const SectionBase *Lhs,
+                                            const SectionBase *Rhs) const {
+  return (sectionPhysicalAddr(Lhs) & 0xFFFFFFFFU) <
+         (sectionPhysicalAddr(Rhs) & 0xFFFFFFFFU);
+}
+
+uint64_t IHexWriter::writeEntryPointRecord(uint8_t *Buf) {
+  IHexLineData HexData;
+  uint8_t Data[4] = {};
+  // We don't write entry point record if entry is zero.
+  if (Obj.Entry == 0)
+    return 0;
+
+  if (Obj.Entry <= 0xFFFFFU) {
+    Data[0] = ((Obj.Entry & 0xF0000U) >> 12) & 0xFF;
+    support::endian::write(&Data[2], static_cast<uint16_t>(Obj.Entry),
+                           support::big);
+    HexData = IHexRecord::getLine(IHexRecord::StartAddr80x86, 0, Data);
+  } else {
+    support::endian::write(Data, static_cast<uint32_t>(Obj.Entry),
+                           support::big);
+    HexData = IHexRecord::getLine(IHexRecord::StartAddr, 0, Data);
+  }
+  memcpy(Buf, HexData.data(), HexData.size());
+  return HexData.size();
+}
+
+uint64_t IHexWriter::writeEndOfFileRecord(uint8_t *Buf) {
+  IHexLineData HexData = IHexRecord::getLine(IHexRecord::EndOfFile, 0, {});
+  memcpy(Buf, HexData.data(), HexData.size());
+  return HexData.size();
+}
+
+Error IHexWriter::write() {
+  IHexSectionWriter Writer(Buf);
+  // Write sections.
+  for (const SectionBase *Sec : Sections)
+    Sec->accept(Writer);
+
+  uint64_t Offset = Writer.getBufferOffset();
+  // Write entry point address.
+  Offset += writeEntryPointRecord(Buf.getBufferStart() + Offset);
+  // Write EOF.
+  Offset += writeEndOfFileRecord(Buf.getBufferStart() + Offset);
+  assert(Offset == TotalSize);
+  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 [%p, %p] 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);
+  };
+  auto IsInPtLoad = [](const SectionBase &Sec) {
+    return Sec.ParentSegment && Sec.ParentSegment->Type == ELF::PT_LOAD;
+  };
+
+  // We can't write 64-bit addresses.
+  if (addressOverflows32bit(Obj.Entry))
+    return createStringError(errc::invalid_argument,
+                             "Entry point address %p overflows 32 bits.",
+                             Obj.Entry);
+
+  // If any section we're to write has segment then we
+  // switch to using physical addresses. Otherwise we
+  // use section virtual address.
+  for (auto &Section : Obj.sections())
+    if (ShouldWrite(Section) && IsInPtLoad(Section)) {
+      UseSegments = true;
+      break;
+    }
+
+  for (auto &Section : Obj.sections())
+    if (ShouldWrite(Section) && (!UseSegments || IsInPtLoad(Section))) {
+      if (Error E = checkSection(Section))
+        return E;
+      Sections.insert(&Section);
+    }
+
+  IHexSectionWriterBase LengthCalc(Buf);
+  for (const SectionBase *Sec : Sections)
+    Sec->accept(LengthCalc);
+
+  // We need space to write section records + StartAddress record
+  // (if start adress is not zero) + EndOfFile record.
+  TotalSize = LengthCalc.getBufferOffset() +
+              (Obj.Entry ? IHexRecord::getLineLength(4) : 0) +
+              IHexRecord::getLineLength(0);
+  if (Error E = Buf.allocate(TotalSize))
+    return E;
+  return Error::success();
+}
+
 template class ELFBuilder<ELF64LE>;
 template class ELFBuilder<ELF64BE>;
 template class ELFBuilder<ELF32LE>;
 template class ELFBuilder<ELF32BE>;
 
 template class ELFWriter<ELF64LE>;
 template class ELFWriter<ELF64BE>;
 template class ELFWriter<ELF32LE>;
 template class ELFWriter<ELF32BE>;
 
 } // end namespace elf
 } // end namespace objcopy
 } // end namespace llvm