[PATCH 2/2] gold: check if the current command line matches the one in the incremental inputs section

[Mikolaj Zalewski <mikolajz at google dot com>]

  This patch adds some sanity checking of .gnu_incremental_inputs
header and checks if the command line in the output file matches the
current command line.

elfcpp:

2009-09-11  Mikolaj Zalewski  <mikolajz@google.com>

      * elf_file.h: (Elf_strtab): New class.

gold:

2009-09-11  Mikolaj Zalewski  <mikolajz@google.com>

      * gold.cc: (queue_initial_tasks): Pass incremental_inputs to
Incremental_checker.
      * incremental.cc: (INCREMENTAL_LINK_VERSION): Change type to unsigned int.
      (Incremental_inputs_header): New class.
      (Incremental_inputs_header_writer): Edit comment.
      (Incremental_inputs_entry): New class.
      (Incremental_inputs_entry_writer): Edit comment.
      (Sized_incremental_binary::do_find_incremental_inputs_section):
Add *strtab_shndx parameter, fill it.
      (Sized_incremental_binary::do_check_inputs): New method.
      (Incremental_checker::can_incrementally_link_output_file): Use
Sized_incremental_binary::check_inputs.
      (Incremental_inputs::report_command_line): Save command line in
command_line_.
      * incremental.h:
(Incremental_binary::find_incremental_inputs_section): New method.
      (Incremental_binary::do_find_incremental_inputs_section): Add
strtab_shndx parameter.
      (Incremental_binary::do_check_inputs): New pure virtual method.
      (Sized_incremental_binary::do_check_inputs): Declare.
      (Incremental_checker::Incremental_checker): Add
incremental_inputs parameter, use it to initialize
incremental_inputs_.
      (Incremental_checker::incremental_inputs_): New field.
      (Incremental_checker::command_line): New method.
      (Incremental_checker::inputs): New method.
      (Incremental_checker::command_line_): New field.

From e5b25750b6c4ebc3ddbbbeb5f18210e42eb86178 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Miko=C5=82aj=20Zalewski?= <mikolajz@tygrys.dom>
Date: Fri, 18 Sep 2009 19:05:41 +0200
Subject: [PATCH] gold: check if the command line matches the incremental inputs section

---
 elfcpp/elfcpp_file.h |   40 +++++++++++++
 gold/gold.cc         |    3 +-
 gold/incremental.cc  |  157 ++++++++++++++++++++++++++++++++++++++++++++++----
 gold/incremental.h   |   62 +++++++++++++++++--
 4 files changed, 242 insertions(+), 20 deletions(-)

diff --git a/elfcpp/elfcpp_file.h b/elfcpp/elfcpp_file.h
index aff1a23..87ee8d7 100644
--- a/elfcpp/elfcpp_file.h
+++ b/elfcpp/elfcpp_file.h
@@ -226,6 +226,35 @@ class Elf_file
   int large_shndx_offset_;
 };
 
+// A small wrapper around SHT_STRTAB data mapped to memory. It checks that the
+// index is not out of bounds and the string is NULL-terminated.
+
+class Elf_strtab
+{
+ public:
+
+  // Construct an Elf_strtab for a section with contents *P and size SIZE.
+  Elf_strtab(const unsigned char* p, size_t size);
+
+  // Return the file offset to the section headers.
+  bool
+  get_c_string(size_t offset, const char** cstring) const
+  {
+    if (offset >= this->usable_size_)
+      return false;
+    *cstring = this->base_ + offset;
+    return true;
+  }
+
+ private:
+
+  // Contents of the section mapped to memory.
+  const char* base_;
+  // One larger that the position of the last NULL character in the section.
+  // For valid SHT_STRTAB sections, this is the size of the section.
+  size_t usable_size_;
+};
+
 // Inline function definitions.
 
 // Check for presence of the ELF magic.
@@ -635,6 +664,17 @@ Elf_file<size, big_endian, File>::section_addralign(unsigned int shndx)
   return shdr.get_sh_addralign();
 }
 
+inline
+Elf_strtab::Elf_strtab(const unsigned char* p, size_t size)
+{
+  // Check if the section is NULL-terminated. If it isn't, we ignore the last
+  // part to make sure we don't return non-NULL-terminated strings.
+  while (size > 0 && p[size - 1] != 0)
+    size--;
+  this->base_ = reinterpret_cast<const char*>(p);
+  this->usable_size_ = size;
+}
+
 } // End namespace elfcpp.
 
 #endif // !defined(ELFCPP_FILE_H)
diff --git a/gold/gold.cc b/gold/gold.cc
index cb29e7d..359a2e2 100644
--- a/gold/gold.cc
+++ b/gold/gold.cc
@@ -180,7 +180,8 @@ queue_initial_tasks(const General_options& options,
   if (cmdline.options().incremental())
     {
       Incremental_checker incremental_checker(
-          parameters->options().output_file_name());
+          parameters->options().output_file_name(),
+          layout->incremental_inputs());
       if (incremental_checker.can_incrementally_link_output_file())
         {
           // TODO: remove when incremental linking implemented.
diff --git a/gold/incremental.cc b/gold/incremental.cc
index 724fc9d..025de5b 100644
--- a/gold/incremental.cc
+++ b/gold/incremental.cc
@@ -37,7 +37,7 @@ namespace gold {
 
 // Version information. Will change frequently during the development, later
 // we could think about backward (and forward?) compatibility.
-const int INCREMENTAL_LINK_VERSION = 1;
+const unsigned int INCREMENTAL_LINK_VERSION = 1;
 
 namespace internal {
 
@@ -84,7 +84,42 @@ struct Incremental_inputs_entry_data
 
 // Accessors.
 
-// See internal::Incremental_input_header for fields descriptions.
+// Reader class for .gnu_incremental_inputs header. See
+// internal::Incremental_input_header for fields descriptions.
+
+template<int size, bool big_endian>
+class Incremental_inputs_header
+{
+ public:
+  Incremental_inputs_header(const unsigned char *p)
+    : p_(reinterpret_cast<const internal::Incremental_inputs_header_data*>(p))
+  { }
+
+  static const int data_size = sizeof(internal::Incremental_inputs_header_data);
+
+  elfcpp::Elf_Word
+  get_version() const
+  { return Convert<32, big_endian>::convert_host(this->p_->version); }
+
+  elfcpp::Elf_Word
+  get_input_file_count() const
+  { return Convert<32, big_endian>::convert_host(this->p_->input_file_count); }
+
+  elfcpp::Elf_Word
+  get_command_line_offset() const
+  { return Convert<32, big_endian>::convert_host(this->p_->command_line_offset); }
+
+  elfcpp::Elf_Word
+  get_reserved() const
+  { return Convert<32, big_endian>::convert_host(this->p_->reserved); }
+
+ private:
+  const internal::Incremental_inputs_header_data* p_;
+};
+
+// Writer class for .gnu_incremental_inputs header. See
+// internal::Incremental_input_header for fields descriptions.
+
 template<int size, bool big_endian>
 class Incremental_inputs_header_write
 {
@@ -115,7 +150,48 @@ class Incremental_inputs_header_write
   internal::Incremental_inputs_header_data* p_;
 };
 
-// See internal::Incremental_input_entry for fields descriptions.
+// Reader class for an .gnu_incremental_inputs entry. See
+// internal::Incremental_input_entry for fields descriptions.
+template<int size, bool big_endian>
+class Incremental_inputs_entry
+{
+ public:
+  Incremental_inputs_entry(const unsigned char *p)
+    : p_(reinterpret_cast<const internal::Incremental_inputs_entry_data*>(p))
+  { }
+
+  static const int data_size = sizeof(internal::Incremental_inputs_entry_data);
+
+  elfcpp::Elf_Word
+  get_filename_offset(elfcpp::Elf_Word v)
+  { return Convert<32, big_endian>::convert_host(this->p_->filename_offset); }
+
+  elfcpp::Elf_Word
+  get_data_offset(elfcpp::Elf_Word v)
+  { return Convert<32, big_endian>::convert_host(this->p_->data_offset); }
+
+  elfcpp::Elf_Xword
+  get_timestamp_sec(elfcpp::Elf_Xword v)
+  { return Convert<64, big_endian>::convert_host(this->p_->timestamp_sec); }
+
+  elfcpp::Elf_Word
+  get_timestamp_nsec(elfcpp::Elf_Word v)
+  { return Convert<32, big_endian>::convert_host(this->p_->timestamp_nsec); }
+
+  elfcpp::Elf_Word
+  get_input_type(elfcpp::Elf_Word v)
+  { return Convert<32, big_endian>::convert_host(this->p_->input_type); }
+
+  elfcpp::Elf_Word
+  get_reserved(elfcpp::Elf_Word v)
+  { return Convert<32, big_endian>::convert_host(this->p_->reserved); }
+
+ private:
+  const internal::Incremental_inputs_entry_data* p_;
+};
+
+// Writer class for an .gnu_incremental_inputs entry. See
+// internal::Incremental_input_entry for fields descriptions.
 template<int size, bool big_endian>
 class Incremental_inputs_entry_write
 {
@@ -196,16 +272,76 @@ Incremental_binary::error(const char* format, ...) const
 template<int size, bool big_endian>
 bool
 Sized_incremental_binary<size, big_endian>::do_find_incremental_inputs_section(
-    Location* location)
+    Location* location,
+    unsigned int* strtab_shndx)
 {
   unsigned int shndx = this->elf_file_.find_section_by_type(
       elfcpp::SHT_GNU_INCREMENTAL_INPUTS);
   if (shndx == elfcpp::SHN_UNDEF)  // Not found.
     return false;
+  *strtab_shndx = this->elf_file_.section_link(shndx);
   *location = this->elf_file_.section_contents(shndx);
   return true;
 }
 
+template<int size, bool big_endian>
+bool
+Sized_incremental_binary<size, big_endian>::do_check_inputs(
+    Incremental_inputs* incremental_inputs)
+{
+  const int entry_size =
+      Incremental_inputs_entry_write<size, big_endian>::data_size;
+  const int header_size =
+      Incremental_inputs_header_write<size, big_endian>::data_size;
+
+  unsigned int strtab_shndx;
+  Location location;
+
+  if (!do_find_incremental_inputs_section(&location, &strtab_shndx))
+    {
+      explain_no_incremental(_("no incremental data from previous build"));
+      return false;
+    }
+  if (location.data_size < header_size ||
+      strtab_shndx >= this->elf_file_.shnum() ||
+      this->elf_file_.section_type(strtab_shndx) != elfcpp::SHT_STRTAB)
+    {
+      explain_no_incremental(_("invalid incremental build data"));
+      return false;
+    }
+
+  Location strtab_location(this->elf_file_.section_contents(strtab_shndx));
+  View data_view(view(location));
+  View strtab_view(view(strtab_location));
+  elfcpp::Elf_strtab strtab(strtab_view.data(), strtab_location.data_size);
+  Incremental_inputs_header<size, big_endian> header(data_view.data());
+
+  if (header.get_version() != INCREMENTAL_LINK_VERSION)
+    {
+      explain_no_incremental(_("different version of incremental build data"));
+      return false;
+    }
+
+  const char* command_line;
+  // We divide instead of multiplying to make sure there is no integer overflow.
+  size_t max_input_entries = (location.data_size - header_size) / entry_size;
+  if (header.get_input_file_count() > max_input_entries ||
+      !strtab.get_c_string(header.get_command_line_offset(), &command_line))
+    {
+      explain_no_incremental(_("invalid incremental build data"));
+      return false;
+    }
+
+  if (incremental_inputs->command_line() != command_line)
+    {
+      explain_no_incremental(_("command line changed"));
+      return false;
+    }
+
+  // TODO: compare incremental_inputs->inputs() with entries in data_view.
+  return true;
+}
+
 namespace
 {
 
@@ -322,13 +458,7 @@ Incremental_checker::can_incrementally_link_output_file()
   std::auto_ptr<Incremental_binary> binary(open_incremental_binary(&output));
   if (binary.get() == NULL)
     return false;
-  Incremental_binary::Location inputs_location;
-  if (!binary->find_incremental_inputs_section(&inputs_location))
-    {
-      explain_no_incremental("no incremental data from previous build");
-      return false;
-    }
-  return true;
+  return binary->check_inputs(this->incremental_inputs_);
 }
 
 // Add the command line to the string table, setting
@@ -365,7 +495,10 @@ Incremental_inputs::report_command_line(int argc, const char* const* argv)
         }
       args.append("'");
     }
-  this->strtab_->add(args.c_str(), true, &this->command_line_key_);
+
+  this->command_line_ = args;
+  this->strtab_->add(this->command_line_.c_str(), false,
+                     &this->command_line_key_);
 }
 
 // Record that the input argument INPUT is an achive ARCHIVE.  This is
diff --git a/gold/incremental.h b/gold/incremental.h
index 8909324..77eb08b 100644
--- a/gold/incremental.h
+++ b/gold/incremental.h
@@ -119,18 +119,34 @@ class Incremental_binary
   void
   error(const char* format, ...) const ATTRIBUTE_PRINTF_2;
 
-
   // Find the .gnu_incremental_inputs section.  It selects the first section
   // of type SHT_GNU_INCREMENTAL_INPUTS.  Returns false if such a section
   // is not found.
   bool
-  find_incremental_inputs_section(Location* location)
-  { return do_find_incremental_inputs_section(location); }
+  find_incremental_inputs_section(Location* location,
+                                  unsigned int* strtab_shndx)
+  { return do_find_incremental_inputs_section(location, strtab_shndx); }
+
+  // Check the .gnu_incremental_inputs section to see whether an incremental
+  // build is possible.
+  // TODO: on success, should report what files needs to be rebuilt.
+  // INCREMENTAL_INPUTS is used to read the canonical form of the command line
+  // and read the input arguments.  TODO: for items that don't need to be
+  // rebuilt, we should also copy the incremental input information.
+  virtual bool
+  check_inputs(Incremental_inputs* incremental_inputs)
+  { return do_check_inputs(incremental_inputs); }
 
  protected:
   // Find incremental inputs section.
   virtual bool
-  do_find_incremental_inputs_section(Location* location) = 0;
+  do_find_incremental_inputs_section(Location* location,
+                                     unsigned int* strtab_shndx) = 0;
+
+  // Check the .gnu_incremental_inputs section to see whether an incremental
+  // build is possible.
+  virtual bool
+  do_check_inputs(Incremental_inputs* incremental_inputs) = 0;
 
  private:
   // Edited output file object.
@@ -151,7 +167,11 @@ class Sized_incremental_binary : public Incremental_binary
 
  protected:
   virtual bool
-  do_find_incremental_inputs_section(Location* location);
+  do_find_incremental_inputs_section(Location* location,
+                                     unsigned int* strtab_shndx);
+
+  virtual bool
+  do_check_inputs(Incremental_inputs* incremental_inputs);
 
  private:
   // Output as an ELF file.
@@ -168,8 +188,14 @@ open_incremental_binary(Output_file* file);
 class Incremental_checker
 {
  public:
-  Incremental_checker(const char* output_name)
-    : output_name_(output_name)
+  // Check if the file named OUTPUT_NAME can be linked incrementally.
+  // INCREMENTAL_INPUTS must have the canonical form of the command line
+  // and input arguments filled - at this point of linking other fields are
+  // probably not filled yet.  TODO: for inputs that don't need to be
+  // rebuilt, this function should fill the incremental input information.
+  Incremental_checker(const char* output_name,
+                      Incremental_inputs* incremental_inputs)
+    : output_name_(output_name), incremental_inputs_(incremental_inputs)
   { }
 
   // Analyzes the output file to check if incremental linking is possible and
@@ -178,7 +204,13 @@ class Incremental_checker
   can_incrementally_link_output_file();
 
  private:
+
+  // Name of the output file to analyze.
   const char* output_name_;
+
+  // The Incremental_inputs object. At this stage of link, only the command
+  // line and inputs are filled.
+  Incremental_inputs* incremental_inputs_;
 };
 
 // This class contains the information needed during an incremental
@@ -227,6 +259,17 @@ class Incremental_inputs
   get_stringpool()
   { return this->strtab_; }
 
+  // Return the canonical form of the command line, as will be stored in
+  // .gnu_incremental_strtab.
+  const std::string&
+  command_line()
+  { return this->command_line_; }
+
+  // Return the input files found in the command line.
+  const Input_arguments*
+  inputs()
+  { return this->inputs_; }
+
  private:
   // Code for each of the four possible variants of create_inputs_section_data.
   template<int size, bool big_endian>
@@ -289,8 +332,13 @@ class Incremental_inputs
   // A map containing additional information about the input elements.
   Inputs_info_map inputs_map_;
 
+  // Canonical form of the command line, as will be stored in
+  // .gnu_incremental_strtab.
+  std::string command_line_;
+
   // The key of the command line string in the string pool.
   Stringpool::Key command_line_key_;
+
   // The .gnu_incremental_strtab string pool associated with the
   // .gnu_incremental_inputs.
   Stringpool* strtab_;
-- 
1.6.0.2