[PDB] Fix unsigned integer overflow

[thakis]

Summary

When building with -fsanitize=unsigned-integer-overflow, this code
causes a diagnostic like:

../../llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp:159:15:
runtime error: unsigned integer overflow:
90 - 229 cannot be represented in type 'unsigned long'

unsigned integer overflow is well defined and it isn't an issue in
practice, but in obscure scenarios (S1.size() small, S2.size over 2GB
on 32-bit systems) it could even be a bug.

So use the usual idiom for implementing cmp functions instead of the
gernally considered buggy idiom :)
See e.g. https://www.gnu.org/software/libc/manual/html_node/Comparison-Functions.html
or https://stackoverflow.com/questions/10996418/efficient-integer-compare-function/10997428#10997428

[hans]

(I've written and been bit by this bug at least one time that I remember. I wonder if clang could have warning tailored to it..)

[thakis]

In D97557#2590412, @hans wrote:

(I've written and been bit by this bug at least one time that I remember. I wonder if clang could have warning tailored to it..)

(I guess you'd pattern-match for if (a_with_some_int_type != b_with_some_int_type) return a_with_some_int_type - b_with_some_int_type; and warn on that and propose a diff like in this patch as fixit? could work!)

llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp

 }
 
 // See `caseInsensitiveComparePchPchCchCch` in gsi.cpp
 static int gsiRecordCmp(StringRef S1, StringRef S2) {
   size_t LS = S1.size();
   size_t RS = S2.size();
   // Shorter strings always compare less than longer strings.
   if (LS != RS)
-    return LS - RS;
+    return (LS > RS) - (LS < RS);
 
   // If either string contains non ascii characters, memcmp them.
   if (LLVM_UNLIKELY(!isAsciiString(S1) || !isAsciiString(S2)))
     return memcmp(S1.data(), S2.data(), LS);
 
   // Both strings are ascii, perform a case-insensitive comparison.
   return S1.compare_lower(S2.data());
 }