[libc++] Fix bogus integer sanitizer warnings in hash helpers #146715

jcelerier · 2025-07-02T15:00:27Z

No description provided.

github-actions · 2025-07-02T15:00:44Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-07-02T15:01:14Z

@llvm/pr-subscribers-libcxx

Author: Jean-Michaël Celerier (jcelerier)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/146715.diff

2 Files Affected:

(modified) libcxx/include/__config (+2)
(modified) libcxx/include/__functional/hash.h (+3-2)

diff --git a/libcxx/include/__config b/libcxx/include/__config
index af8a297fdf3fd..5bbb54f94ae23 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -1156,8 +1156,10 @@ typedef __char32_t char32_t;
 // Allow for build-time disabling of unsigned integer sanitization
 #  if __has_attribute(no_sanitize) && !defined(_LIBCPP_COMPILER_GCC)
 #    define _LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK __attribute__((__no_sanitize__("unsigned-integer-overflow")))
+#    define _LIBCPP_DISABLE_UBSAN_INTEGER_CHECK __attribute__((__no_sanitize__("integer")))
 #  else
 #    define _LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK
+#    define _LIBCPP_DISABLE_UBSAN_INTEGER_CHECK
 #  endif
 
 #  if __has_feature(nullability)
diff --git a/libcxx/include/__functional/hash.h b/libcxx/include/__functional/hash.h
index 489a6f00b8a3d..5c4fa39c772f5 100644
--- a/libcxx/include/__functional/hash.h
+++ b/libcxx/include/__functional/hash.h
@@ -132,11 +132,12 @@ struct __murmur2_or_cityhash<_Size, 64> {
   static const _Size __k2 = 0x9ae16a3b2f90404fULL;
   static const _Size __k3 = 0xc949d7c7509e6557ULL;
 
-  _LIBCPP_HIDE_FROM_ABI static _Size __rotate(_Size __val, int __shift) {
+  _LIBCPP_HIDE_FROM_ABI static _Size _LIBCPP_DISABLE_UBSAN_INTEGER_CHECK __rotate(_Size __val, int __shift) {
     return __shift == 0 ? __val : ((__val >> __shift) | (__val << (64 - __shift)));
   }
 
-  _LIBCPP_HIDE_FROM_ABI static _Size __rotate_by_at_least_1(_Size __val, int __shift) {
+  _LIBCPP_HIDE_FROM_ABI static _Size _LIBCPP_DISABLE_UBSAN_INTEGER_CHECK
+  __rotate_by_at_least_1(_Size __val, int __shift) {
     return (__val >> __shift) | (__val << (64 - __shift));
   }

EricWF · 2025-07-02T22:46:59Z

Could you provide the UBSAN diagnostic you're seeing and trying to disable?

ldionne

Like @EricWF , I'd like to understand what this is silencing, but I don't have an objection assuming the rationale makes sense!

libcxx/include/__functional/hash.h

jcelerier · 2025-07-03T22:17:45Z

here's for instance the messages I'm getting: (values come from a call I had in one of my ubsan backtraces)

#include <iostream>
using _Size = unsigned long;

static _Size __rotate(_Size __val, int __shift) {
    return __shift == 0 ? __val : ((__val >> __shift) | (__val << (64 - __shift)));
  }

int main() {
  std::cout << __rotate(17751278790833232267U, 43) << "\n";
}

$ clang++ -Wextra -Wall   foo.cpp -fsanitize=undefined -fsanitize=address -fsanitize=integer -g && ./a.out
foo.cpp:5:64: runtime error: left shift of 17751278790833232267 by 21 places cannot be represented in type '_Size' (aka 'unsigned long')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior foo.cpp:5:64

I tried disabling only "shift" instead of the wholesale "integer" but it didn't fix the warning

jcelerier · 2025-07-03T22:20:41Z

here's for instance all the warnings I get whenever I start my app just coming from this place:


/usr/bin/../include/c++/v1/__functional/hash.h:133:64: runtime error: left shift of 17751278790833232267 by 21 places cannot be represented in type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../include/c++/v1/__functional/hash.h:133:64 
/usr/bin/../include/c++/v1/__functional/hash.h:137:40: runtime error: left shift of 7738135736995832184 by 55 places cannot be represented in type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../include/c++/v1/__functional/hash.h:137:40 
/usr/bin/../include/c++/v1/__functional/hash.h:137:40: runtime error: left shift of 5642809484591583822 by 53 places cannot be represented in type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../include/c++/v1/__functional/hash.h:137:40 
/usr/bin/../include/c++/v1/__functional/hash.h:137:40: runtime error: left shift of 8314892262689629553 by 50 places cannot be represented in type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../include/c++/v1/__functional/hash.h:137:40 
/usr/bin/../include/c++/v1/__functional/hash.h:137:40: runtime error: left shift of 8461812133216675204 by 48 places cannot be represented in type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../include/c++/v1/__functional/hash.h:137:40 
/usr/bin/../include/c++/v1/__functional/hash.h:133:64: runtime error: left shift of 13621769879915339144 by 21 places cannot be represented in type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../include/c++/v1/__functional/hash.h:133:64 
/usr/bin/../include/c++/v1/__functional/hash.h:137:40: runtime error: left shift of 7809631459534666091 by 54 places cannot be represented in type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../include/c++/v1/__functional/hash.h:137:40 
/usr/bin/../include/c++/v1/__functional/hash.h:133:64: runtime error: left shift of 8773319506221311228 by 21 places cannot be represented in type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../include/c++/v1/__functional/hash.h:133:64 
/usr/bin/../include/c++/v1/__functional/hash.h:133:64: runtime error: left shift of 18129537700234053450 by 12 places cannot be represented in type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../include/c++/v1/__functional/hash.h:133:64 
/usr/bin/../include/c++/v1/__functional/hash.h:133:64: runtime error: left shift of 15310047686210979080 by 21 places cannot be represented in type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../include/c++/v1/__functional/hash.h:133:64 
/usr/bin/../include/c++/v1/__functional/hash.h:133:64: runtime error: left shift of 17062917676620572615 by 21 places cannot be represented in type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../include/c++/v1/__functional/hash.h:133:64 
/usr/bin/../include/c++/v1/__functional/hash.h:137:40: runtime error: left shift of 8389209267074581617 by 52 places cannot be represented in type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../include/c++/v1/__functional/hash.h:137:40

whenever I start lldb I have to sift through all of those which gets fairly frustrating quickly..

jcelerier · 2025-07-03T22:32:37Z

string repro:

#include <string>
#include <functional>

int main() { std::hash<std::string>{}( "EnableActionIfDocument"); }

philnik777 · 2025-07-04T08:55:34Z

Can we enable the sanitizer in our CI, so this doesn't regress?

ldionne

This makes sense to me. Just to make sure we're on the same page: the mathematical result of this left shift is indeed not representable in an unsigned long, but that's okay because we discard the bits that are shifted beyond the "end" of the integer. So we have:

__val << (64 - __shift)

where __val = 17751278790833232267U and __shift = 43, giving 17751278790833232267U << 21. That number would be 37302001761581901365411840 which doesn't fit inside an unsigned long, but in reality what we get is just the lower 64 bits of that number, aka 5770268451330048.

So the operation itself is valid and the UBSan warning is bogus. If I got that right, then I'm fine with this patch.

I do agree with @philnik777 that we should enable this in our sanitizer CI. This should be doable with something like this patch:

diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index dffdd7a3c70a..9bbd64cce370 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -611,10 +611,10 @@ function(get_sanitizer_flags OUT_VAR  USE_SANITIZER)
         append_flags(SANITIZER_FLAGS "-fsanitize-memory-track-origins")
       endif()
     elseif (USE_SANITIZER STREQUAL "Undefined")
-      append_flags(SANITIZER_FLAGS "-fsanitize=undefined" "-fno-sanitize=vptr,function" "-fno-sanitize-recover=all")
+      append_flags(SANITIZER_FLAGS "-fsanitize=undefined,integer" "-fno-sanitize=vptr,function" "-fno-sanitize-recover=all")
     elseif (USE_SANITIZER STREQUAL "Address;Undefined" OR
             USE_SANITIZER STREQUAL "Undefined;Address")
-      append_flags(SANITIZER_FLAGS "-fsanitize=address,undefined" "-fno-sanitize=vptr,function" "-fno-sanitize-recover=all")
+      append_flags(SANITIZER_FLAGS "-fsanitize=address,undefined,integer" "-fno-sanitize=vptr,function" "-fno-sanitize-recover=all")
     elseif (USE_SANITIZER STREQUAL "Thread")
       append_flags(SANITIZER_FLAGS -fsanitize=thread)
     elseif (USE_SANITIZER STREQUAL "DataFlow")
diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index fc34009d0a55..65715117e7dc 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -317,8 +317,8 @@ DEFAULT_PARAMETERS = [
             [
                 AddFlag("-g -fno-omit-frame-pointer") if sanitizer else None,
 
-                AddFlag("-fsanitize=undefined -fno-sanitize=float-divide-by-zero -fno-sanitize-recover=all") if sanitizer == "Undefined" else None,
-                AddFeature("ubsan")                                                                          if sanitizer == "Undefined" else None,
+                AddFlag("-fsanitize=undefined,integer -fno-sanitize=float-divide-by-zero -fno-sanitize-recover=all") if sanitizer == "Undefined" else None,
+                AddFeature("ubsan")                                                                                  if sanitizer == "Undefined" else None,
 
                 AddFlag("-fsanitize=address") if sanitizer == "Address" else None,
                 AddFeature("asan")            if sanitizer == "Address" else None,

That will bundle -fsanitize=integer into our existing ubsan checks, but I think that should be reasonable?

jcelerier requested a review from a team as a code owner July 2, 2025 15:00

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jul 2, 2025

jcelerier mentioned this pull request Jul 2, 2025

__config: also ignore shift-out-of-bounds error with ubsan checks #146688

Closed

ldionne changed the title ~~libcxx: fix bogus integer sanitizer warnings in hash helpers~~ [libc++] Fix bogus integer sanitizer warnings in hash helpers Jul 3, 2025

ldionne reviewed Jul 3, 2025

View reviewed changes

libcxx/include/__functional/hash.h Outdated Show resolved Hide resolved

libcxx: fix bogus integer sanitizer warnings in hash helpers

f04051d

jcelerier force-pushed the main branch from 3c3712a to f04051d Compare July 3, 2025 22:35

ldionne reviewed Jul 4, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++] Fix bogus integer sanitizer warnings in hash helpers #146715

[libc++] Fix bogus integer sanitizer warnings in hash helpers #146715

jcelerier commented Jul 2, 2025

Uh oh!

github-actions bot commented Jul 2, 2025

Uh oh!

llvmbot commented Jul 2, 2025

Uh oh!

EricWF commented Jul 2, 2025

Uh oh!

ldionne left a comment

Uh oh!

Uh oh!

jcelerier commented Jul 3, 2025

Uh oh!

jcelerier commented Jul 3, 2025

Uh oh!

jcelerier commented Jul 3, 2025

Uh oh!

philnik777 commented Jul 4, 2025

Uh oh!

ldionne left a comment

Uh oh!

Uh oh!

[libc++] Fix bogus integer sanitizer warnings in hash helpers #146715

Are you sure you want to change the base?

[libc++] Fix bogus integer sanitizer warnings in hash helpers #146715

Conversation

jcelerier commented Jul 2, 2025

Uh oh!

github-actions bot commented Jul 2, 2025

Uh oh!

llvmbot commented Jul 2, 2025

Uh oh!

EricWF commented Jul 2, 2025

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jcelerier commented Jul 3, 2025

Uh oh!

jcelerier commented Jul 3, 2025

Uh oh!

jcelerier commented Jul 3, 2025

Uh oh!

philnik777 commented Jul 4, 2025

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!