Skip to content

Conversation

@vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Dec 11, 2024

nullptr + offset is possible after !is_virtual branch.

Detected with check-cxxabi on configured with:

cmake -DLLVM_APPEND_VC_REV=OFF -GNinja \
  -DCMAKE_BUILD_TYPE=Release \
  -DLLVM_CCACHE_BUILD=ON \
  -DLLVM_USE_LINKER=lld \
  -DLLVM_ENABLE_ASSERTIONS=ON \
  -DCMAKE_C_COMPILER=clang \
  -DCMAKE_CXX_COMPILER=clang++ \
  -DLIBCXXABI_USE_LLVM_UNWINDER=OFF \
  -DCMAKE_INSTALL_PREFIX=/home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/libcxx_install_ubsan \
  '-DLLVM_ENABLE_RUNTIMES=libcxx;libcxxabi;libunwind' \
  -DLIBCXX_TEST_PARAMS=long_tests=False \
  -DLIBCXX_INCLUDE_BENCHMARKS=OFF \
  -DLLVM_USE_SANITIZER=Undefined \
  '-DCMAKE_C_FLAGS=-fsanitize=undefined -fno-sanitize-recover=all   -fno-sanitize=vptr' \
  '-DCMAKE_CXX_FLAGS=-fsanitize=undefined -fno-sanitize-recover=all   -fno-sanitize=vptr' \
  /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/../runtimes

********************
Failed Tests (2):
  llvm-libc++abi-shared.cfg.in :: catch_null_pointer_to_object_pr64953.pass.cpp
  llvm-libc++abi-shared.cfg.in :: catch_ptr_02.pass.cpp

Created using spr 1.3.4
@vitalybuka vitalybuka requested a review from a team as a code owner December 11, 2024 07:55
@llvmbot llvmbot added the libc++abi libc++abi C++ Runtime Library. Not libc++. label Dec 11, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 11, 2024

@llvm/pr-subscribers-libcxxabi

Author: Vitaly Buka (vitalybuka)

Changes

nullptr + offset is possible after !is_virtual branch.

Fixes https://lab.llvm.org/buildbot/#/builders/85/builds/3200/steps/10/logs/stdio


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

1 Files Affected:

  • (modified) libcxxabi/src/private_typeinfo.cpp (+1-1)
diff --git a/libcxxabi/src/private_typeinfo.cpp b/libcxxabi/src/private_typeinfo.cpp
index 2f631041f74c94..8f6e8c6631de4c 100644
--- a/libcxxabi/src/private_typeinfo.cpp
+++ b/libcxxabi/src/private_typeinfo.cpp
@@ -593,7 +593,7 @@ __base_class_type_info::has_unambiguous_public_base(__dynamic_cast_info* info,
   }
     __base_type->has_unambiguous_public_base(
             info,
-            static_cast<char*>(adjustedPtr) + offset_to_base,
+            reinterpret_cast<void*>(reinterpret_cast<uintptr_t>(adjustedPtr) + offset_to_base),
             (__offset_flags & __public_mask) ? path_below : not_public_path);
 }
 

Created using spr 1.3.4
@vitalybuka vitalybuka requested a review from MaskRay December 11, 2024 07:57
vitalybuka added a commit to llvm/llvm-zorg that referenced this pull request Dec 11, 2024
vitalybuka added a commit to llvm/llvm-zorg that referenced this pull request Dec 11, 2024
@MaskRay
Copy link
Member

MaskRay commented Dec 11, 2024

Fixes lab.llvm.org/buildbot#/builders/85/builds/3200/steps/10/logs/stdio

The build bot link will expire. Can you replace this with concrete configuration?
I guess it is when building libc++abi with -fsanitize=pointer-overflow (part of -fsanitize=undefined)

@ldionne
Copy link
Member

ldionne commented Dec 11, 2024

@vitalybuka Why don't we run into this issue in our -fsanitize=undefined existing (pre-commit) CI bot?

@vitalybuka
Copy link
Collaborator Author

vitalybuka commented Dec 11, 2024

@vitalybuka Why don't we run into this issue in our -fsanitize=undefined existing (pre-commit) CI bot?

I detected this after adding -fno-sanitize-recover=all to our bots, by default ubsan just prints a warning and moves on.
So probably CI missing this as well

@vitalybuka
Copy link
Collaborator Author

vitalybuka commented Dec 11, 2024

@vitalybuka Why don't we run into this issue in our -fsanitize=undefined existing (pre-commit) CI bot?

I detected this after adding -fno-sanitize-recover=all to our bots, by default ubsan just prints a warning and moves on. So probably CI missing this as well

@ldionne
Actually problem is that if we only rely on -DLLVM_USE_SANITIZER=Undefined
libcxxabi/src/private_typeinfo.cpp compiles without -fsanitize=undefined at all
I think entire cxxabi. And it does not look intentional as there is some DLLVM_USE_SANITIZER stuff in cmake files.

#119612

@vitalybuka vitalybuka merged commit a54fce8 into main Dec 11, 2024
60 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/libcabi-dont-do-pointer-arithmetic-on-nullptr branch December 11, 2024 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++abi libc++abi C++ Runtime Library. Not libc++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants