[SourceManager] Support buffers that are not null-terminated

[loladiro]

Summary

I feed custom buffers to clang, and since the buffers store an explicit length, I did not think to null-terminate them.
However, this causes clang to run out-of-bounds in certain situations in ComputeLineNumbers. Since under certain
situations ComputeLineNumbers does look at the given bounds, I figured the proper solution was to always check the
buffer for EOF. Please let me know if I missed something and this was intended.

[loladiro]

Bump. Could somebody take a look at this?

[klimek]

Generally, I thought clang often relies on buffers being null terminated to speed up parse times. Usually the MemoryBuffers have an option to guarantee null-terminatedness (and copy if necessary)

[loladiro]

Hmm, you're right. And I am actually constructing it in such a way that it's supposed to have it, so I wonder why it ran off the rails here. Will take another look.

[loladiro]

Oh, I see it checks for the null terminator past the end of the given memory block on construction, but if the memory after changes later this just keeps running. Well, that's quite a trap, but I guess that's what you get for not understanding the API ;).

lib/Basic/SourceManager.cpp

#ifdef __SSE2__
     // Try to skip to the next newline using SSE instructions. This is very
     // performance sensitive for programs with lots of diagnostics and in -E
     // mode.
     __m128i CRs = _mm_set1_epi8('\r');
     __m128i LFs = _mm_set1_epi8('\n');
 
     // First fix up the alignment to 16 bytes.
     while (((uintptr_t)NextBuf & 0xF) != 0) {
-      if (*NextBuf == '\n' || *NextBuf == '\r' || *NextBuf == '\0')
+      if (*NextBuf == '\n' || *NextBuf == '\r' || *NextBuf == '\0' || NextBuf == End)
         goto FoundSpecialChar;
       ++NextBuf;
     }
 
     // Scan 16 byte chunks for '\r' and '\n'. Ignore '\0'.
     while (NextBuf+16 <= End) {
       const __m128i Chunk = *(const __m128i*)NextBuf;
       __m128i Cmp = _mm_or_si128(_mm_cmpeq_epi8(Chunk, CRs),
                                  _mm_cmpeq_epi8(Chunk, LFs));
       unsigned Mask = _mm_movemask_epi8(Cmp);
 
       // If we found a newline, adjust the pointer and jump to the handling code.
       if (Mask != 0) {
         NextBuf += llvm::countTrailingZeros(Mask);
         goto FoundSpecialChar;
       }
       NextBuf += 16;
     }
 #endif
 
-    while (*NextBuf != '\n' && *NextBuf != '\r' && *NextBuf != '\0')
+    while (*NextBuf != '\n' && *NextBuf != '\r' && *NextBuf != '\0' && NextBuf == End)
       ++NextBuf;
 
 #ifdef __SSE2__
 FoundSpecialChar:
 #endif
     Offs += NextBuf-Buf;
     Buf = NextBuf;
 
+    // If end of file, exit.
+    if (Buf == End) break;
+
     if (Buf[0] == '\n' || Buf[0] == '\r') {
       // If this is \n\r or \r\n, skip both characters.
       if ((Buf[1] == '\n' || Buf[1] == '\r') && Buf[0] != Buf[1])
         ++Offs, ++Buf;
       ++Offs, ++Buf;
       LineOffsets.push_back(Offs);
     } else {
-      // Otherwise, this is a null.  If end of file, exit.
-      if (Buf == End) break;
       // Otherwise, skip the null.
       ++Offs, ++Buf;
     }
   }
 
   // Copy the offsets into the FileInfo structure.
   FI->NumLines = LineOffsets.size();
   FI->SourceLineCache = Alloc.Allocate<unsigned>(LineOffsets.size());