tree 7a8b8a410b320ad21b48bc0910906e4916073c4c
parent 8c231cd10d8ad8d5437190717dfc5d5968e139d4
author Ryan Prichard <rprichard@google.com> 1557797820 -0700
committer Ryan Prichard <rprichard@google.com> 1557866147 -0700

Refactor deps log loading, fix 2 bugs

There were two bugs in DepsLog::Load's main parsing pass:

 * Previously, with an invalid log file, the main pass could initialize
   dep_index[output_id] with the index of a record after the point where
   the log is truncated, e.g.:

    - Chunk 1: path record for node #0
    - Chunk 1: invalid record
    - Chunk 2: path record for node #1
    - Chunk 2: deps record outputting node #0, needs node #1

   The result of the parse could depend on chunk boundaries (e.g. how many
   threads the machine has), and the parser could crash if the later deps
   record has source IDs that were also truncated.

   Fix the problem by moving dep_index initialization to a later pass. The
   validation and truncation work is factored out into a ValidateDepsLog
   function.

 * Fix node ID validation of deps record inputs. The existing code to do
   this had no effect:

        if (output_id < 0 || output_id >= next_node_id) break;
        for (size_t i = 4; i < size; ++i) {
          int input_id = log.table[index + i];
          if (input_id < 0 || input_id >= next_node_id) break;

   The outer break exited the for-each-record loop early, signaling that
   parsing had failed. The nested break exited the for-each-input loop,
   which merely prevented validation of later node IDs. Replace the break
   statements with "return false" in IsValidRecord.

These two changes regressed ".ninja_deps load" run-time by about 10ms
on a 580MB .ninja_deps file. (e.g. about 140ms -> 150ms). I suspect the
compiler may have been optimizing out the source ID checking.

Test: ninja_test
Change-Id: I13c3a314cfa7d2bf15c724962a9ec35f55176779