[PATCH] D41742: [MSF] Fix FPM interval calculation

[Zachary Turner via Phabricator via llvm-commits]

zturner created this revision.
zturner added reviewers: rnk, colden.
Herald added a subscriber: hiraditya.

The FPM (free page map) in an MSF file is an odd structure.  It is a series of blocks that are spread throughout the file at periodic intervals.  And there are actually two separate FPMs organized in the file this way.

The first one, `FPM0`, begins at block 1 and a new piece occurs every `BlockSize` blocks.
The second one, `FPM1`, begins at block 2 and a new piece also occurs every `BlockSize` blocks.

Graphically, this can be represented like this:

  Block Index |  0  |   1   |   2   |  ...  ┊  4096  |  4097  |  4098  | ... | 
      Meaning | SB  |  FPM0 | FPM1  | Data  ┊  Data  |  FPM0  |  FPM1  | ... |

where ┊ indicates a a period-boundary.

We had an off by 1 error where if there were exactly 4097 blocks (or, for that matter, 4096 * n + 1 blocks for any n), one piece of code would decide "oh that ends before FPM0 from this period, so we don't have an FPM0 to worry about" whereas another piece of code would say "there are two FPM intervals here" and then when iterating over those 2 intervals, would access uninitialized memory since the other piece of code only returned data from 1.

The fix here is to properly handle these edge cases.  If block count is of the form 4096 * n + 1, we properly return n-1 fpm intervals.

There's another odd edge case that wasn't being triggered, but was found by inspection when looking into this issue.  We shouldn't ever create a PDB file with 4098 blocks, because that would look like this:

  Block Index |  0  |   1   |   2   |  ...  ┊  4096  |  4097  |
      Meaning | SB  |  FPM0 | FPM1  | Data  ┊  Data  |  FPM0  |

and here we've got a continuation of FPM0 but not of FPM1.  Although we can promise never to generate one of these, we probably should have the code handle it gracefully anyway and not crash (since we can't control what we see).

Initial investigative work and patch was done by Colden Cullen in https://reviews.llvm.org/D41734 (there's also some interesting discussion over there if anyone wants to follow)


https://reviews.llvm.org/D41742

Files:
  llvm/include/llvm/DebugInfo/MSF/MSFCommon.h
  llvm/lib/DebugInfo/MSF/MSFCommon.cpp
  llvm/unittests/DebugInfo/MSF/MSFCommonTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41742.128656.patch
Type: text/x-patch
Size: 5041 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180104/a58f48df/attachment.bin>