[PATCH] D91693: [Support] Add reserve() method to the raw_ostream.

Alexey Lapshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 1 14:36:36 PST 2020


avl added a comment.

> Honestly, I don't have the time to look at this process in detail, and refactoring things to support an objcopy library is not high on my priority list. I'm not convinced that your suggestions are actually going to be workable/useful in practice, if I'm honest, and have tried to outline my concerns already.

yes. thank you for your comments. I tried to explain why I think that streams could still be used. Please, check whether it is correct argument.

> My biggest concern is how do you stream an ELF header without already knowing where your section header table will live.

please, check three ways of how to do this:

1. Use preliminary memory buffer(ELF header could be updated after sections are written):



  executeObjcopyOnBinary(CopyConfig &Config, object::Binary &In, raw_ostream &Out) {
    Buf = WritableMemoryBuffer::getNewMemBuffer(totalSize()); <<< allocate preliminary memory buffer
    
    // Following code stores result in the buffer Buf(that is how the current implementation works).
    // Segment data must be written first, so that the ELF header and program
    // header tables can overwrite it, if covered by a segment.
    writeSegmentData();
    writeEhdr();
    writePhdrs();
    if (Error E = writeSectionData())
      return E;
    if (WriteSectionHeaders)
      writeShdrs();
  
    Out.write(Buf->getBufferStart(), Buf->getBufferSize()); <<<<< copy data from internal buffer into output stream
    Out.flush();
    return Error::success();  
  }

That scheme looks equal(in the sence of copied output data) to solution you have proposed earlier:

  void aFunction(...) {
    ...
    Object Obj = executeObjcopy(Config, In);  <<<< allocate memory buffer and return result in it
    size_t Size = Obj.getOutputSize();
    ...
    raw_fd_ostream Out(FileName);
    ...
    writeOutput(Out, Obj);  <<< copy data from the memory buffer into output file
  }

2. Write directly into the stream:

Before writing started the overall output file size is already known. 
It is calculated by ELFWriter<ELFT>::finalize(). Probably, the information about the place where section header table will live could be understood inside ELFWriter<ELFT>::finalize() ? In that case the ELF header
might be written immediately in the correct form.

3. If above two variants are not good then we could consider using raw_pwrite_stream for the destination.



  executeObjcopyOnBinary(CopyConfig &Config, object::Binary &In, raw_pwrite_stream &Out) {
  
    // Following code stores result in the raw_pwrite_stream &Out.
    // Segment data must be written first, so that the ELF header and program
    // header tables can overwrite it, if covered by a segment.
    writeSegmentData();
    writeEhdr();
    writePhdrs();
    if (Error E = writeSectionData())
      return E;
    if (WriteSectionHeaders)
      writeShdrs();
  
    // seek to the ELF header and update it
    Out.seek(to ELF Header);
    Out.write(updated ELF header);
  
    return Error::success();  
  }

> if you know where your section header table will live, you have all the information you need for presizing your output buffer, so being able to reserve post stream creation becomes pointless.

Why it becomes pointless?

> There's no need to read the entire input object file into memory either - llvm-objcopy doesn't do this already (note that generic sections that require no manipulation just use an ArrayRef to refer to the section contents).

Right. I am not proposing to optimize memory used for input object file.
That proposal is about using streams to use standard classes and to optimize memory used for the output file.

following is an example when the entire output file is loaded into the memory:

  Expected<std::unique_ptr<MemoryBuffer>>
  object::writeUniversalBinaryToBuffer(ArrayRef<Slice> Slices) {
    SmallVector<char, 0> Buffer;
    raw_svector_ostream Out(Buffer);
  
    if (Error E = writeUniversalBinaryToStream(Slices, Out))  <<< entire output file is loaded into the memory
      return std::move(E);
      
    return std::make_unique<SmallVectorMemoryBuffer>(std::move(Buffer));
  }    
  ......    
    Expected<std::unique_ptr<MemoryBuffer>> B =
        writeUniversalBinaryToBuffer(Slices);
    if (!B)
      return B.takeError();
    if (Error E = Out.allocate((*B)->getBufferSize()))
      return E;
    memcpy(Out.getBufferStart(), (*B)->getBufferStart(), (*B)->getBufferSize());  
    ^^^^^^^^^
    data is copied from memory into the output buffer


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91693/new/

https://reviews.llvm.org/D91693



More information about the llvm-commits mailing list