Fix code comments for {CodeReference} based on {CodeDesc}.

R=sigurds@chromium.org
BUG=v8:9089

Change-Id: I6092ff322588e42e83251464b8a4c61ad0324384
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1559860
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60743}

src/code-comments.cc

@@ -27,16 +27,19 @@ uint32_t CodeCommentEntry::size() const {
   return kOffsetToCommentString + comment_length();
 }
 
-CodeCommentsIterator::CodeCommentsIterator(Address code_comments_start)
+CodeCommentsIterator::CodeCommentsIterator(Address code_comments_start,
+                                           uint32_t code_comments_size)
     : code_comments_start_(code_comments_start),
-      current_entry_(code_comments_start + kOffsetToFirstCommentEntry) {}
-
-uint32_t CodeCommentsIterator::size() const {
-  return code_comments_start_ != kNullAddress
-             ? *reinterpret_cast<uint32_t*>(code_comments_start_)
-             : 0;
+      code_comments_size_(code_comments_size),
+      current_entry_(code_comments_start + kOffsetToFirstCommentEntry) {
+  DCHECK_NE(kNullAddress, code_comments_start);
+  DCHECK_IMPLIES(
+      code_comments_size,
+      code_comments_size == *reinterpret_cast<uint32_t*>(code_comments_start_));
 }
 
+uint32_t CodeCommentsIterator::size() const { return code_comments_size_; }
+
 const char* CodeCommentsIterator::GetComment() const {
   const char* comment_string =
       reinterpret_cast<const char*>(current_entry_ + kOffsetToCommentString);
@@ -84,8 +87,9 @@ uint32_t CodeCommentsWriter::section_size() const {
   return kOffsetToFirstCommentEntry + static_cast<uint32_t>(byte_count_);
 }
 
-void PrintCodeCommentsSection(std::ostream& out, Address code_comments_start) {
-  CodeCommentsIterator it(code_comments_start);
+void PrintCodeCommentsSection(std::ostream& out, Address code_comments_start,
+                              uint32_t code_comments_size) {
+  CodeCommentsIterator it(code_comments_start, code_comments_size);
   out << "CodeComments (size = " << it.size() << ")\n";
   if (it.HasCurrent()) {
     out << std::setw(6) << "pc" << std::setw(6) << "len"

src/code-comments.h

@@ -20,7 +20,7 @@ class Assembler;
 // Code comments section layout:
 // byte count              content
 // ------------------------------------------------------------------------
-// 4                       size as uint32_t
+// 4                       size as uint32_t (only for sanity check)
 // [Inline array of CodeCommentEntry in increasing pc_offset order]
 // ┌ 4                     pc_offset of entry as uint32_t
 // ├ 4                     length of the comment including terminating '\0'
@@ -47,8 +47,8 @@ class CodeCommentsWriter {
 
 class V8_EXPORT_PRIVATE CodeCommentsIterator {
  public:
-  // Address can be kNullAddress. In this case HasCurrent() will return false.
-  explicit CodeCommentsIterator(Address code_comments_start);
+  CodeCommentsIterator(Address code_comments_start,
+                       uint32_t code_comments_size);
   uint32_t size() const;
   const char* GetComment() const;
   uint32_t GetCommentSize() const;
@@ -58,10 +58,12 @@ class V8_EXPORT_PRIVATE CodeCommentsIterator {
 
  private:
   Address code_comments_start_;
+  uint32_t code_comments_size_;
   Address current_entry_;
 };
 
-void PrintCodeCommentsSection(std::ostream& out, Address code_comments_start);
+void PrintCodeCommentsSection(std::ostream& out, Address code_comments_start,
+                              uint32_t code_comments_size);
 
 }  // namespace internal
 }  // namespace v8

src/code-reference.cc

@@ -25,6 +25,7 @@ struct JSOps {
   const byte* relocation_end() const { return code->relocation_end(); }
   int relocation_size() const { return code->relocation_size(); }
   Address code_comments() const { return code->code_comments(); }
+  int code_comments_size() const { return code->code_comments_size(); }
 };
 
 struct WasmOps {
@@ -45,6 +46,7 @@ struct WasmOps {
   }
   int relocation_size() const { return code->reloc_info().length(); }
   Address code_comments() const { return code->code_comments(); }
+  int code_comments_size() const { return code->code_comments_size(); }
 };
 
 struct CodeDescOps {
@@ -70,6 +72,7 @@ struct CodeDescOps {
   Address code_comments() const {
     return instruction_start() + code_desc->code_comments_offset;
   }
+  int code_comments_size() const { return code_desc->code_comments_size; }
 };
 }  // namespace
 
@@ -96,6 +99,7 @@ DISPATCH(const byte*, relocation_start)
 DISPATCH(const byte*, relocation_end)
 DISPATCH(int, relocation_size)
 DISPATCH(Address, code_comments)
+DISPATCH(int, code_comments_size)
 
 #undef DISPATCH
 

src/code-reference.h

@@ -35,6 +35,7 @@ class CodeReference {
   const byte* relocation_end() const;
   int relocation_size() const;
   Address code_comments() const;
+  int code_comments_size() const;
 
   bool is_null() const { return kind_ == NONE; }
   bool is_js() const { return kind_ == JS; }

src/disassembler.cc

@@ -276,7 +276,7 @@ static int DecodeIt(Isolate* isolate, ExternalReferenceEncoder* ref_encoder,
   disasm::Disassembler d(converter,
                          disasm::Disassembler::kContinueOnUnimplementedOpcode);
   RelocIterator* it = nullptr;
-  CodeCommentsIterator cit(code.code_comments());
+  CodeCommentsIterator cit(code.code_comments(), code.code_comments_size());
   // Relocation exists if we either have no isolate (wasm code),
   // or we have an isolate and it is not an off-heap instruction stream.
   if (!isolate ||

src/heap/code-stats.cc

@@ -202,7 +202,7 @@ void CodeStatistics::CollectCodeCommentStatistics(HeapObject obj,
   }
 
   Code code = Code::cast(obj);
-  CodeCommentsIterator cit(code->code_comments());
+  CodeCommentsIterator cit(code->code_comments(), code->code_comments_size());
   int delta = 0;
   int prev_pc_offset = 0;
   while (cit.HasCurrent()) {

src/objects/code-inl.h

@@ -557,7 +557,6 @@ Address Code::constant_pool() const {
 }
 
 Address Code::code_comments() const {
-  if (!has_code_comments()) return kNullAddress;
   return InstructionStart() + code_comments_offset();
 }
 

src/objects/code.cc

@@ -798,7 +798,7 @@ void Code::Disassemble(const char* name, std::ostream& os, Address current_pc) {
   }
 
   if (has_code_comments()) {
-    PrintCodeCommentsSection(os, code_comments());
+    PrintCodeCommentsSection(os, code_comments(), code_comments_size());
   }
 }
 #endif  // ENABLE_DISASSEMBLER

src/objects/code.h

@@ -167,7 +167,7 @@ class Code : public HeapObject {
   inline int code_comments_offset() const;
   inline void set_code_comments_offset(int offset);
   inline Address code_comments() const;
-  int code_comments_size() const;
+  V8_EXPORT_PRIVATE int code_comments_size() const;
   V8_EXPORT_PRIVATE bool has_code_comments() const;
 
   // The size of the executable instruction area, without embedded metadata.

src/wasm/wasm-code-manager.cc

@@ -109,10 +109,12 @@ Address WasmCode::constant_pool() const {
 }
 
 Address WasmCode::code_comments() const {
-  if (code_comments_offset_ < unpadded_binary_size_) {
-    return instruction_start() + code_comments_offset_;
-  }
-  return kNullAddress;
+  return instruction_start() + code_comments_offset_;
+}
+
+uint32_t WasmCode::code_comments_size() const {
+  DCHECK_GE(unpadded_binary_size_, code_comments_offset_);
+  return static_cast<uint32_t>(unpadded_binary_size_ - code_comments_offset_);
 }
 
 size_t WasmCode::trap_handler_index() const {
@@ -339,10 +341,8 @@ void WasmCode::Disassemble(const char* name, std::ostream& os,
   }
   os << "\n";
 
-  if (code_comments_offset() < unpadded_binary_size_) {
-    Address code_comments = reinterpret_cast<Address>(instructions().start() +
-                                                      code_comments_offset());
-    PrintCodeCommentsSection(os, code_comments);
+  if (code_comments_size() > 0) {
+    PrintCodeCommentsSection(os, code_comments(), code_comments_size());
   }
 #endif  // ENABLE_DISASSEMBLER
 }

src/wasm/wasm-code-manager.h

@@ -115,6 +115,7 @@ class V8_EXPORT_PRIVATE WasmCode final {
   ExecutionTier tier() const { return tier_; }
   Address constant_pool() const;
   Address code_comments() const;
+  uint32_t code_comments_size() const;
   size_t constant_pool_offset() const { return constant_pool_offset_; }
   size_t safepoint_table_offset() const { return safepoint_table_offset_; }
   size_t handler_table_offset() const { return handler_table_offset_; }

test/cctest/compiler/test-code-assembler.cc

@@ -594,7 +594,7 @@ TEST(TestCodeAssemblerCodeComment) {
 
   Handle<Code> code = asm_tester.GenerateCode();
   CHECK_NE(code->code_comments(), kNullAddress);
-  CodeCommentsIterator it(code->code_comments());
+  CodeCommentsIterator it(code->code_comments(), code->code_comments_size());
   CHECK(it.HasCurrent());
   bool found_comment = false;
   while (it.HasCurrent()) {