[PATCH] D97557: [PDB] Fix unsigned integer overflow

Nico Weber via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 26 08:19:30 PST 2021


thakis created this revision.
thakis added a reviewer: hans.
Herald added a subscriber: hiraditya.
thakis requested review of this revision.
Herald added a project: LLVM.

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


https://reviews.llvm.org/D97557

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


Index: llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp
===================================================================
--- llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp
+++ llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp
@@ -156,7 +156,7 @@
   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)))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97557.326705.patch
Type: text/x-patch
Size: 551 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210226/a8c87136/attachment.bin>


More information about the llvm-commits mailing list