GetMedianTimePast doesn't incorporate its own block #10755

BIP9 specifies

GetMedianTimePast in the code ... refers to the median nTime of a block and its 10 predecessors.

However, in src/chain.h, GetMedianTimePast is defined as

    int64_t GetMedianTimePast() const
    {
        int64_t pmedian[nMedianTimeSpan];
        int64_t* pbegin = &pmedian[nMedianTimeSpan];
        int64_t* pend = &pmedian[nMedianTimeSpan];

        const CBlockIndex* pindex = this;
        for (int i = 0; i < nMedianTimeSpan && pindex; i++, pindex = pindex->pprev)
            *(--pbegin) = pindex->GetBlockTime();

        std::sort(pbegin, pend);
        return pbegin[(pend - pbegin)/2];
    }

This implementation omits the block itself (i.e. this->GetBlockTime()) from consideration, since we're decrementing pbegin before its first use. This leaves pmedian[nMedianTimeSpan] unassigned. This is almost never an issue because we end up selecting the median element from the range, but calling this method on the genesis block would produce undefined behavior AFAICT.

This omission may well be intentional, but I figured I'd verify that here. If it isn't intentional, I'll file a PR containing the following change along with some supporting unittests:

diff --git a/src/chain.h b/src/chain.h
index c5304b7..fbe92b1 100644
--- a/src/chain.h
+++ b/src/chain.h
@@ -313,6 +313,8 @@ public:
         int64_t* pend = &pmedian[nMedianTimeSpan];
 
         const CBlockIndex* pindex = this;
+        *pbegin = pindex->GetBlockTime();
+
         for (int i = 0; i < nMedianTimeSpan && pindex; i++, pindex = pindex->pprev)
             *(--pbegin) = pindex->GetBlockTime();

After writing a small unittest, I've realized there's no issue with the existing code. Just an embarrassing lapse in pointer comprehension on my end. Oops!