structured comments (#7192)

src/annotated_binary_text_gen.cpp

@@ -3,6 +3,8 @@
 #include <sstream>
 #include <string>
 
+#include "binary_annotator.h"
+#include "flatbuffers/base.h"
 #include "flatbuffers/util.h"
 
 namespace flatbuffers {
@@ -54,7 +56,7 @@ template<typename T>
 std::string ToValueString(const BinaryRegion &region, const uint8_t *binary) {
   std::string s;
   s += "0x";
-  const T val = GetScalar<T>(binary + region.offset);
+  const T val = ReadScalar<T>(binary + region.offset);
   const uint64_t start_index = region.offset + region.length - 1;
   for (uint64_t i = 0; i < region.length; ++i) {
     s += ToHex(binary[start_index - i]);
@@ -149,8 +151,113 @@ static std::string GenerateTypeString(const BinaryRegion &region) {
               : "");
 }
 
+static std::string GenerateComment(const BinaryRegionComment &comment,
+                                   const BinarySection &) {
+  std::string s;
+  switch (comment.type) {
+    case BinaryRegionCommentType::Unknown: s = "unknown"; break;
+    case BinaryRegionCommentType::SizePrefix: s = "size prefix"; break;
+    case BinaryRegionCommentType::RootTableOffset:
+      s = "offset to root table `" + comment.name + "`";
+      break;
+    // TODO(dbaileychess): make this lowercase to follow the convention.
+    case BinaryRegionCommentType::FileIdentifier: s = "File Identifier"; break;
+    case BinaryRegionCommentType::Padding: s = "padding"; break;
+    case BinaryRegionCommentType::VTableSize: s = "size of this vtable"; break;
+    case BinaryRegionCommentType::VTableRefferingTableLength:
+      s = "size of referring table";
+      break;
+    case BinaryRegionCommentType::VTableFieldOffset:
+      s = "offset to field `" + comment.name;
+      break;
+    case BinaryRegionCommentType::VTableUnknownFieldOffset:
+      s = "offset to unknown field (id: " + std::to_string(comment.index) + ")";
+      break;
+
+    case BinaryRegionCommentType::TableVTableOffset:
+      s = "offset to vtable";
+      break;
+    case BinaryRegionCommentType::TableField:
+      s = "table field `" + comment.name;
+      break;
+    case BinaryRegionCommentType::TableUnknownField: s = "unknown field"; break;
+    case BinaryRegionCommentType::TableOffsetField:
+      s = "offset to field `" + comment.name + "`";
+      break;
+    case BinaryRegionCommentType::StructField:
+      s = "struct field `" + comment.name + "`";
+      break;
+    case BinaryRegionCommentType::ArrayField:
+      s = "array field `" + comment.name + "`[" +
+          std::to_string(comment.index) + "]";
+      break;
+    case BinaryRegionCommentType::StringLength: s = "length of string"; break;
+    case BinaryRegionCommentType::StringValue: s = "string literal"; break;
+    case BinaryRegionCommentType::StringTerminator:
+      s = "string terminator";
+      break;
+    case BinaryRegionCommentType::VectorLength:
+      s = "length of vector (# items)";
+      break;
+    case BinaryRegionCommentType::VectorValue:
+      s = "value[" + std::to_string(comment.index) + "]";
+      break;
+    case BinaryRegionCommentType::VectorTableValue:
+      s = "offset to table[" + std::to_string(comment.index) + "]";
+      break;
+    case BinaryRegionCommentType::VectorStringValue:
+      s = "offset to string[" + std::to_string(comment.index) + "]";
+      break;
+    case BinaryRegionCommentType::VectorUnionValue:
+      s = "offset to union[" + std::to_string(comment.index) + "]";
+      break;
+
+    default: break;
+  }
+  if (!comment.default_value.empty()) { s += " " + comment.default_value; }
+
+  switch (comment.status) {
+    case BinaryRegionStatus::OK: break;  // no-op
+    case BinaryRegionStatus::WARN: s = "WARN: " + s; break;
+    case BinaryRegionStatus::WARN_NO_REFERENCES:
+      s = "WARN: nothing refers to this section.";
+      break;
+    case BinaryRegionStatus::WARN_CORRUPTED_PADDING:
+      s = "WARN: could be corrupted padding region.";
+      break;
+    case BinaryRegionStatus::WARN_PADDING_LENGTH:
+      s = "WARN: padding is longer than expected.";
+      break;
+    case BinaryRegionStatus::ERROR: s = "ERROR: " + s; break;
+    case BinaryRegionStatus::ERROR_OFFSET_OUT_OF_BINARY:
+      s = "ERROR: " + s + ". Invalid offset, points outside the binary.";
+      break;
+    case BinaryRegionStatus::ERROR_INCOMPLETE_BINARY:
+      s = "ERROR: " + s + ". Incomplete binary, expected to read " +
+          comment.status_message + " bytes.";
+      break;
+    case BinaryRegionStatus::ERROR_LENGTH_TOO_LONG:
+      s = "ERROR: " + s + ". Longer than the binary.";
+      break;
+    case BinaryRegionStatus::ERROR_LENGTH_TOO_SHORT:
+      s = "ERROR: " + s + ". Shorter than the minimum length: ";
+      break;
+    case BinaryRegionStatus::ERROR_REQUIRED_FIELD_NOT_PRESENT:
+      s = "ERROR: " + s + ". Required field is not present.";
+      break;
+    case BinaryRegionStatus::ERROR_INVALID_UNION_TYPE:
+      s = "ERROR: " + s + ". Invalid union type value.";
+      break;
+    case BinaryRegionStatus::ERROR_CYCLE_DETECTED:
+      s = "ERROR: " + s + ". Invalid offset, cycle detected.";
+      break;
+  }
+
+  return s;
+}
+
 static std::string GenerateDocumentation(const BinaryRegion &region,
-                                         const BinarySection &,
+                                         const BinarySection &section,
                                          const uint8_t *binary,
                                          DocContinuation &continuation,
                                          const OutputConfig &output_config) {
@@ -199,11 +306,9 @@ static std::string GenerateDocumentation(const BinaryRegion &region,
   }
 
   s += " ";
-  if (!region.comment.empty()) {
-    s += output_config.delimiter;
-    s += " ";
-    s += region.comment;
-  }
+  s += output_config.delimiter;
+  s += " ";
+  s += GenerateComment(region.comment, section);
 
   return s;
 }

src/annotated_binary_text_gen.h

@@ -52,8 +52,9 @@ class AnnotatedBinaryTextGenerator {
         options_(options) {}
 
   // Generate the annotated binary for the given `filename`. Returns true if the
-  // annotated binary was succesfully saved.
-  bool Generate(const std::string &filename, const std::string &schema_filename);
+  // annotated binary was successfully saved.
+  bool Generate(const std::string &filename,
+                const std::string &schema_filename);
 
  private:
   const std::map<uint64_t, BinarySection> annotations_;

src/binary_annotator.h

@@ -48,10 +48,6 @@ enum class BinaryRegionType {
   UType = 17,
 };
 
-template<typename T> static inline T GetScalar(const uint8_t *binary) {
-  return *reinterpret_cast<const T *>(binary);
-}
-
 template<typename T>
 static inline std::string ToHex(T i, size_t width = sizeof(T)) {
   std::stringstream stream;
@@ -65,6 +61,89 @@ static inline std::string ToHex(uint8_t i) {
   return ToHex(static_cast<int>(i), 2);
 }
 
+enum class BinaryRegionStatus {
+  OK = 0,
+  WARN = 100,
+  WARN_NO_REFERENCES,
+  WARN_CORRUPTED_PADDING,
+  WARN_PADDING_LENGTH,
+  ERROR = 200,
+  // An offset is pointing outside the binary bounds.
+  ERROR_OFFSET_OUT_OF_BINARY,
+  // Expecting to read N bytes but not enough remain in the binary.
+  ERROR_INCOMPLETE_BINARY,
+  // When a length of a vtable/vector is longer than possible.
+  ERROR_LENGTH_TOO_LONG,
+  // When a length of a vtable/vector is shorter than possible.
+  ERROR_LENGTH_TOO_SHORT,
+  // A field mark required is not present in the vtable.
+  ERROR_REQUIRED_FIELD_NOT_PRESENT,
+  // A realized union type is not within the enum bounds.
+  ERROR_INVALID_UNION_TYPE,
+  // Occurs when there is a cycle in offsets.
+  ERROR_CYCLE_DETECTED,
+};
+
+enum class BinaryRegionCommentType {
+  Unknown = 0,
+  SizePrefix,
+  // The offset to the root table.
+  RootTableOffset,
+  // The optional 4-char file identifier.
+  FileIdentifier,
+  // Generic 0-filled padding
+  Padding,
+  // The size of the vtable.
+  VTableSize,
+  // The size of the referring table.
+  VTableRefferingTableLength,
+  // Offsets to vtable fields.
+  VTableFieldOffset,
+  // Offsets to unknown vtable fields.
+  VTableUnknownFieldOffset,
+  // The vtable offset of a table.
+  TableVTableOffset,
+  // A "inline" table field value.
+  TableField,
+  // A table field that is unknown.
+  TableUnknownField,
+  // A table field value that points to another section.
+  TableOffsetField,
+  // A struct field value.
+  StructField,
+  // A array field value.
+  ArrayField,
+  // The length of the string.
+  StringLength,
+  // The string contents.
+  StringValue,
+  // The explicit string terminator.
+  StringTerminator,
+  // The length of the vector (# of items).
+  VectorLength,
+  // A "inline" value of a vector.
+  VectorValue,
+  // A vector value that points to another section.
+  VectorTableValue,
+  VectorStringValue,
+  VectorUnionValue,
+};
+
+struct BinaryRegionComment {
+  BinaryRegionStatus status = BinaryRegionStatus::OK;
+
+  // If status is non OK, this may be filled in with additional details.
+  std::string status_message;
+
+  BinaryRegionCommentType type = BinaryRegionCommentType::Unknown;
+
+  std::string name;
+
+  std::string default_value;
+
+  size_t index = 0;
+};
+
 struct BinaryRegion {
   // Offset into the binary where this region begins.
   uint64_t offset = 0;
@@ -84,9 +163,7 @@ struct BinaryRegion {
   uint64_t points_to_offset = 0;
 
   // The comment on the region.
-  // TODO(dbaileychess): Consider moving this to a more structure comment field
-  // so that other generators can parse it easier.
-  std::string comment;
+  BinaryRegionComment comment;
 };
 
 enum class BinarySectionType {
@@ -221,7 +298,7 @@ class BinaryAnnotator {
   }
 
   inline bool IsValidRead(const uint64_t offset, const uint64_t length) const {
-    return IsValidOffset(offset + length - 1);
+    return length < binary_length_ && IsValidOffset(offset + length - 1);
   }
 
   // Calculate the number of bytes remaining from the given offset. If offset is
@@ -291,9 +368,7 @@ class BinaryAnnotator {
     }
   }
 
-  bool ContainsSection(const uint64_t offset) {
-    return sections_.find(offset) != sections_.end();
-  }
+  bool ContainsSection(const uint64_t offset);
 
   // The schema for the binary file
   const uint8_t *bfbs_;