ART fix oat debug source map operations

Several places need to be fixed in OAT debug source map generation
(see comments in https://android-review.googlesource.com/#/c/102610/19/compiler/compiled_method.h):
1. Source Maps are deduplicated in Compiler Driver by implicit conversion
   SrcMapElems to bytes. This implies incorrect operator==.
2. SrcMapElem operator < is peculiar, and cannot be applied to
   SrcMapElems with negative to_ fields
3. SrcMap.Arrange method is not elegant

The fix is to introduce explicit conversion from SrcMapElem to one
signed 64-bit value, which is used as a base of two new operators < and ==.
They are correct and intuitive. DedupeHashFunc is changed to
explicitly convert array elements to byte, so the explicit type conversion
from SrcMapElem to byte is used.

Minor fix: In Line Table Programs the file index set command is generated
only if the index gets new value.

Change-Id: I5e2c03404a437254fc2db3485b22bfc1799b39b7
Signed-off-by: Yevgeny Rouban <yevgeny.y.rouban@intel.com>

diff --git a/compiler/compiled_method.h b/compiler/compiled_method.h
index d02cbff..36f4745 100644 (file)
--- a/compiler/compiled_method.h
+++ b/compiler/compiled_method.h
@@ -105,59 +105,40 @@ class SrcMapElem {
   uint32_t from_;
   int32_t to_;
 
+  explicit operator int64_t() const {
+    return (static_cast<int64_t>(to_) << 32) | from_;
+  }
+
   bool operator<(const SrcMapElem& sme) const {
-    uint64_t lhs = (static_cast<uint64_t>(from_) << 32) + to_;
-    uint64_t rhs = (static_cast<uint64_t>(sme.from_) << 32) + sme.to_;
-    return lhs < rhs;
+    return int64_t(*this) < int64_t(sme);
+  }
+
+  bool operator==(const SrcMapElem& sme) const {
+    return int64_t(*this) == int64_t(sme);
   }
 
-  operator uint8_t() const {
+  explicit operator uint8_t() const {
     return static_cast<uint8_t>(from_ + to_);
   }
 };
 
 class SrcMap FINAL : public std::vector<SrcMapElem> {
  public:
-  struct CompareByTo {
-    bool operator()(const SrcMapElem& lhs, const SrcMapElem& rhs) {
-      return lhs.to_ < rhs.to_;
-    }
-  };
-
-  struct CompareByFrom {
-    bool operator()(const SrcMapElem& lhs, const SrcMapElem& rhs) {
-      return lhs.from_ < rhs.from_;
-    }
-  };
-
-  void SortByTo() {
-    std::sort(begin(), end(), CompareByTo());
-  }
-
   void SortByFrom() {
-    std::sort(begin(), end(), CompareByFrom());
+    std::sort(begin(), end(), [] (const SrcMapElem& lhs, const SrcMapElem& rhs) -> bool {
+      return lhs.from_ < rhs.from_;
+    });
   }
 
   const_iterator FindByTo(int32_t to) const {
-    return std::lower_bound(begin(), end(), SrcMapElem({0, to}), CompareByTo());
+    return std::lower_bound(begin(), end(), SrcMapElem({0, to}));
   }
 
   SrcMap& Arrange() {
-    SortByTo();
-
-    // Remove duplicate pairs.
     if (!empty()) {
-      SrcMap tmp;
-      tmp.swap(*this);
-      iterator it = tmp.begin();
-      iterator prev = it;
-      it++;
-      push_back(*prev);
-      for (; it != tmp.end(); it++) {
-        if (prev->from_ != it->from_ || prev->to_ != it->to_) {
-          push_back(*(prev = it));
-        }
-      }
+      std::sort(begin(), end());
+      resize(std::unique(begin(), end()) - begin());
+      shrink_to_fit();
     }
     return *this;
   }

diff --git a/compiler/driver/compiler_driver.h b/compiler/driver/compiler_driver.h
index 87523bf..624947d 100644 (file)
--- a/compiler/driver/compiler_driver.h
+++ b/compiler/driver/compiler_driver.h
@@ -766,7 +766,7 @@ class CompilerDriver {
       size_t hash = 0x811c9dc5;
       if (array.size() <= kSmallArrayThreshold) {
         for (auto b : array) {
-          hash = (hash * 16777619) ^ b;
+          hash = (hash * 16777619) ^ static_cast<uint8_t>(b);
         }
       } else {
         // For larger arrays use the 2 bytes at 6 bytes (the location of a push registers
@@ -774,12 +774,12 @@ class CompilerDriver {
         // values at random.
         static const size_t kRandomHashCount = 16;
         for (size_t i = 0; i < 2; ++i) {
-          uint8_t b = array[i + 6];
+          uint8_t b = static_cast<uint8_t>(array[i + 6]);
           hash = (hash * 16777619) ^ b;
         }
         for (size_t i = 2; i < kRandomHashCount; ++i) {
           size_t r = i * 1103515245 + 12345;
-          uint8_t b = array[r % array.size()];
+          uint8_t b = static_cast<uint8_t>(array[r % array.size()]);
           hash = (hash * 16777619) ^ b;
         }
       }

diff --git a/compiler/elf_writer_quick.cc b/compiler/elf_writer_quick.cc
index e45eb61..4d78a3c 100644 (file)
--- a/compiler/elf_writer_quick.cc
+++ b/compiler/elf_writer_quick.cc
@@ -1115,7 +1115,7 @@ class LineTableGenerator FINAL : public Leb128Encoder {
                      size_t current_line)
     : Leb128Encoder(data), line_base_(line_base), line_range_(line_range),
       opcode_base_(opcode_base), current_address_(current_address),
-      current_line_(current_line) {}
+      current_line_(current_line), current_file_index_(0) {}
 
   void PutDelta(unsigned delta_addr, int delta_line) {
     current_line_ += delta_line;
@@ -1169,8 +1169,11 @@ class LineTableGenerator FINAL : public Leb128Encoder {
   }
 
   void SetFile(unsigned file_index) {
-    PushByte(data_, DW_LNS_set_file);
-    PushBackUnsigned(file_index);
+    if (current_file_index_ != file_index) {
+      current_file_index_ = file_index;
+      PushByte(data_, DW_LNS_set_file);
+      PushBackUnsigned(file_index);
+    }
   }
 
   void EndSequence() {
@@ -1187,6 +1190,7 @@ class LineTableGenerator FINAL : public Leb128Encoder {
   const int opcode_base_;
   uintptr_t current_address_;
   size_t current_line_;
+  unsigned current_file_index_;
 
   DISALLOW_COPY_AND_ASSIGN(LineTableGenerator);
 };
@@ -1460,16 +1464,17 @@ void ElfWriterQuick::FillInCFIInformation(OatWriter* oat_writer,
       PushWord(dbg_info, dbg.low_pc_ + text_section_offset);
       PushWord(dbg_info, dbg.high_pc_ + text_section_offset);
 
-      pc2java_map.clear();
       GetLineInfoForJava(dbg.dbgstream_, dbg.compiled_method_->GetSrcMappingTable(),
                          &pc2java_map, dbg.low_pc_);
       pc2java_map.DeltaFormat({dbg.low_pc_, 1}, dbg.high_pc_);
-
-      line_table_generator.SetFile(file_index);
-      line_table_generator.SetAddr(dbg.low_pc_ + text_section_offset);
-      line_table_generator.SetLine(1);
-      for (auto& src_map_elem : pc2java_map) {
-        line_table_generator.PutDelta(src_map_elem.from_, src_map_elem.to_);
+      if (!pc2java_map.empty()) {
+        line_table_generator.SetFile(file_index);
+        line_table_generator.SetAddr(dbg.low_pc_ + text_section_offset);
+        line_table_generator.SetLine(1);
+        for (auto& src_map_elem : pc2java_map) {
+          line_table_generator.PutDelta(src_map_elem.from_, src_map_elem.to_);
+        }
+        pc2java_map.clear();
       }
     }