Reland Implement .eh_frame writer and disassembler.

src/eh-frame.h

Index: src/eh-frame.h
diff --git a/src/eh-frame.h b/src/eh-frame.h
index 75781acb502ee8f420e93b240e7515e6e9f7348d..151deb67c4a529b0845e1a499a662f87a865852b 100644
--- a/src/eh-frame.h
+++ b/src/eh-frame.h
@@ -5,19 +5,175 @@
 #ifndef V8_EH_FRAME_H_
 #define V8_EH_FRAME_H_
 
-#include <cstdint>
+#include "src/macro-assembler.h"
 
 namespace v8 {
 namespace internal {
 
-class Code;
+class EhFrameWriter {
+ public:
+  enum DwarfOpcodes : byte {

[rmcilroy, 2016/07/05 10:56:12]

enum class please

[Stefano Sanfilippo, 2016/07/05 16:02:13]

Done.

+    kNop = 0x00,
+    kAdvanceLoc1 = 0x02,
+    kAdvanceLoc2 = 0x03,
+    kAdvanceLoc4 = 0x04,
+    kSameValue = 0x08,
+    kDefCfa = 0x0c,
+    kDefCfaRegister = 0x0d,
+    kDefCfaOffset = 0x0e,
+    kOffsetExtendedSf = 0x11,
+  };
+
+  enum DwarfEncodingSpecifiers : byte {

[rmcilroy, 2016/07/05 10:56:13]

ditto

[Stefano Sanfilippo, 2016/07/05 16:02:14]

These need to be | combinable though, static_cast<...>(a) | static_cast<...>(b)
does not seem very elegant to me. Any idea?

[rmcilroy, 2016/07/06 13:59:30]

OK, enum is fine in that case.

+    kUData4 = 0x03,
+    kSData4 = 0x0b,
+    kPcRel = 0x10,
+    kDataRel = 0x30,
+    kOmit = 0xff,
+  };
+
+  static const uint32_t kInt32Placeholder = 0xdeadc0de;
+
+  static const int kLocationTag = 1;
+  static const int kSavedRegisterTag = 2;

[rmcilroy, 2016/07/05 10:56:13]

Move SavedRegisterTag below the location mask / mask size and newline between
location and saved register constants

[Stefano Sanfilippo, 2016/07/05 16:02:13]

Done.

+  static const int kLocationMask = 0x3f;
+  static const int kLocationMaskSize = 6;
+  static const int kSavedRegisterMask = 0x3f;
+  static const int kSavedRegisterMaskSize = 6;
+
+  static const int kProcedureAddressOffsetInFde = 2 * kInt32Size;
+  static const int kProcedureSizeOffsetInFde = 3 * kInt32Size;
+  static const int kTerminatorSize = 4;

[rmcilroy, 2016/07/05 10:56:12]

kFdeTerminatorSize ?  also put in it's own block (and move
kInitialStateOffsetInCIE to the top of this bloc).

[Stefano Sanfilippo, 2016/07/05 16:02:13]

Done the moving. About the name, the terminator terminates the entire eh_frame,
maybe kEhFrameTerminatorSize?

[rmcilroy, 2016/07/06 13:59:30]

SGTM

+
+  static const int kInitialStateOffsetInCIE = 19;
+
+  static const int kDataAlignmentFactor;

[rmcilroy, 2016/07/05 10:56:13]

This should all be private, none ofthese fields or enums are used outside of
this class or tests.

[Stefano Sanfilippo, 2016/07/05 16:02:14]

The encoding specifiers are used in the EhFrameHdr, now that I have split
EhFrameDisassembler and EhFrameIterator we also need the others.

[rmcilroy, 2016/07/06 13:59:30]

These shouldn't be part of EhFrameWriter in that case. Perhaps create a separate
class EhFrame for frame specific constants. Maybe this could also replace the
EhFrameHdr class (i.e., move all the EhFrameHeader things into this new EhFrame
class).

[Stefano Sanfilippo, 2016/07/06 16:54:49]

As discussed, I have uploaded a patchset with the EhFrameConstants class, please
let me know your thoughts about it.

+
+  EhFrameWriter();
+
+  void AdvanceLocation(int pc_offset);
+
+  // The <base_address> is the one to which all <offset>s in SaveRegisterToStack
+  // directives are relative. It is given by <base_register> + <base_offset>.
+  void SetBaseAddressRegister(Register base_register);
+  void SetBaseAddressOffset(int base_offset);
+  void IncreaseBaseAddressOffset(int base_delta) {
+    SetBaseAddressOffset(base_offset_ + base_delta);
+  }
+  void SetBaseAddressRegisterAndOffset(Register base_register, int base_offset);
+
+  // Offset relative to <base address>, positive offsets go in the direction
+  // of stack growth (downwards on all architectures we support).
+  void RegisterSavedToStack(Register name, int offset) {
+    RegisterSavedToStack(RegisterToDwarfCode(name), offset);
+  }
+
+  void RegisterSavedToStack(Register name) {
+    RegisterSavedToStack(name, base_offset_);
+  }
+
+  void RegisterIsValid(Register name);
+
+  void Finish(int code_size);
+
+  // Remember to call Finish() before GetEhFrame().
+  //
+  // The EhFrameWriter instance owns the buffer pointed by
+  // CodeDesc::unwinding_info, and must outlive any use of the CodeDesc.
+  void GetEhFrame(CodeDesc* desc);
+
+  int last_pc_offset() const { return last_pc_offset_; }
+  Register base_register() const { return base_register_; }
+  int base_offset() const { return base_offset_; }
+
+#ifdef ENABLE_DISASSEMBLER
+  static void DisassembleToStream(std::ostream& stream,  // NOLINT
+                                  const byte* start, const byte* end);
+#endif
+
+ protected:

[rmcilroy, 2016/07/05 10:56:13]

This should all be private - you've already made EhFrameWriterInternals a friend
class

[Stefano Sanfilippo, 2016/07/05 16:02:14]

Done.

+  static uint32_t DecodeULEB128(const byte* encoded, int* encoded_size);
+

[rmcilroy, 2016/07/05 10:56:13]

nit - remove newline (and group common blocks (e.g., all U/SLEB128 functions -
also make the names complimentary - e.g., Read/Write or Encode/Decode)

[Stefano Sanfilippo, 2016/07/05 16:02:13]

Done.

+  static int32_t DecodeSLEB128(const byte* encoded, int* encoded_size);
+
+  void WriteByte(byte value) { eh_frame_buffer_.push_back(value); }
+
+  void WriteBytes(const byte* start, int size) {
+    eh_frame_buffer_.insert(eh_frame_buffer_.end(), start, start + size);
+  }
+
+  void WriteInt16(uint16_t value) {
+    WriteBytes(reinterpret_cast<const byte*>(&value), sizeof(value));
+  }
+
+  void WriteInt32(uint32_t value) {
+    WriteBytes(reinterpret_cast<const byte*>(&value), sizeof(value));
+  }
+
+  void WriteSLEB128(int32_t value);
+
+  void WriteULEB128(uint32_t value);
+
+  void PatchInt32(int base_offset, uint32_t value) {
+    DCHECK_EQ(ReadUnalignedUInt32(eh_frame_buffer_.data() + base_offset),
+              kInt32Placeholder);
+    DCHECK_LT(base_offset + kInt32Size, eh_frame_offset());
+    WriteUnalignedUInt32(eh_frame_buffer_.data() + base_offset, value);
+  }
+
+  void WriteFDEHeader();

[rmcilroy, 2016/07/05 10:56:13]

Short comments for these more complex functions

[Stefano Sanfilippo, 2016/07/05 16:02:13]

Done.

[rmcilroy, 2016/07/06 13:59:30]

Still no comments?

[Stefano Sanfilippo, 2016/07/06 16:54:49]

Sorry, they came in a later patchset.

+
+  void WriteCIE();
+
+  void WriteInitialState();

[rmcilroy, 2016/07/05 10:56:13]

WriteInitialStateInCIE ?

[Stefano Sanfilippo, 2016/07/05 16:02:14]

Done.

+
+  void WriteReturnAddressRegisterCode();

[rmcilroy, 2016/07/05 10:56:13]

This should just be "Register GetReturnAddressRegister()", and then write it in
the common way (using RegisterToDwarfCode and WriteULEB128)

[Stefano Sanfilippo, 2016/07/05 16:02:14]

The problem is that x64 rip is a pseudoregister with no Register instance
associated. If you do not like this approach, we could either have a
kAddressRegisterCode constant or a GetReturnAddressCode() method that give the
DWARF code for the return address register. 

+
+  void Align();

[rmcilroy, 2016/07/05 10:56:13]

WritePaddingTo8ByteAlign

[Stefano Sanfilippo, 2016/07/05 16:02:13]

Done.

+
+  // Internal version that directly accepts a DWARF register code, needed for
+  // handling pseudoregisters on some platforms, such as x64.
+  void RegisterSavedToStack(int register_code, int offset);
+
+  int eh_frame_offset() const {
+    return static_cast<int>(eh_frame_buffer_.size());
+  }
+
+  int fde_offset() const { return cie_size_; }
+
+  int procedure_address_offset() const {
+    return fde_offset() + kProcedureAddressOffsetInFde;
+  }
+
+  int procedure_size_offset() const {
+    return fde_offset() + kProcedureSizeOffsetInFde;
+  }
+
+ private:
+#ifdef ENABLE_DISASSEMBLER
+  static void DumpDWARFDirectives(std::ostream& stream,  // NOLINT
+                                  const byte* begin, const byte* end);
+#endif
+
+  static const char* DwarfRegisterCodeToString(int code);
+  static int RegisterToDwarfCode(Register name);
+
+  int last_pc_offset_;
+  bool eh_frame_finalised_;
+  Register base_register_;
+  int base_offset_;
+  std::vector<byte> eh_frame_buffer_;
+  int cie_size_;

[rmcilroy, 2016/07/05 10:56:13]

nit - move cie_size_ to be first field

[Stefano Sanfilippo, 2016/07/05 16:02:14]

Done.

+
+  friend class EhFrameWriterInternals;  // For unit tests.

[rmcilroy, 2016/07/05 10:56:13]

Rename this class to EhFrameWriteTest and remove comments
(EhFrameWriterInternals could be a non-test class)

[Stefano Sanfilippo, 2016/07/05 16:02:14]

Removed the friend altogether, since the internals that needed testing have been
moved to other classes and we agreed to only examine the output .eh_frame 

+};
 
 class EhFrameHdr final {
  public:
   static const int kRecordSize = 20;
-  static const int kCIESize;
 
-  explicit EhFrameHdr(Code* code);
+  EhFrameHdr(int code_size, int eh_frame_size, int cie_size);
+  static EhFrameHdr CreateEmptyHeader();
 
   int32_t offset_to_eh_frame() const { return offset_to_eh_frame_; }
   uint32_t lut_entries_number() const { return lut_entries_number_; }
@@ -25,6 +181,10 @@ class EhFrameHdr final {
   int32_t offset_to_fde() const { return offset_to_fde_; }
 
  private:
+  static const int kEhFrameHdrVersion = 1;
+
+  EhFrameHdr() {}
+
   uint8_t version_;
   uint8_t eh_frame_ptr_encoding_;
   uint8_t lut_size_encoding_;