[clang] [compiler-rt] [UBSan] Fix incorrect alignment reported when global new returns an o… (PR #152532)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (gbMattN)

<details>
<summary>Changes</summary>

…ffset pointer

This problem and fix was originally raised on Phabricator, https://reviews.llvm.org/D116861, by belkiss. I wasn't able to find them on github or the discourse so I'm re-raising this. The problem was reported as follows

> I found that ubsan will report an incorrect alignment for a type in case it is allocated with the global operator new (without alignment), if we have it return an offset ptr.
> 
> I wrote a small repro: https://godbolt.org/z/n8Yh8eoaE
> 
> The type is aligned on 8 bytes (verified by static_assert on its alignof), but ubsan reports: "constructor call on misaligned address 0x000002af8fd8 for type 'Param', which requires 16 byte alignment".


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


2 Files Affected:

- (modified) clang/lib/CodeGen/CGExprCXX.cpp (+1-1) 
- (added) compiler-rt/test/ubsan/TestCases/global-new-alignment.cpp (+32) 


``````````diff
diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index c7e53331fe05f..ecc71a6343fd6 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -1798,7 +1798,7 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) {
   SkippedChecks.set(SanitizerKind::Null, nullCheck);
   EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall,
                 E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
-                result, allocType, result.getAlignment(), SkippedChecks,
+                result, allocType, allocAlign, SkippedChecks,
                 numElements);
 
   EmitNewInitializer(*this, E, allocType, elementTy, result, numElements,
diff --git a/compiler-rt/test/ubsan/TestCases/global-new-alignment.cpp b/compiler-rt/test/ubsan/TestCases/global-new-alignment.cpp
new file mode 100644
index 0000000000000..4586a0932affd
--- /dev/null
+++ b/compiler-rt/test/ubsan/TestCases/global-new-alignment.cpp
@@ -0,0 +1,32 @@
+// RUN: %clangxx -fsanitize=alignment %s -o %t
+// RUN: %run %t 2>&1 | FileCheck %s --implicit-check-not="runtime error" -allow-empty
+// Disable with msan and tsan because they also override global new
+// UNSUPPORTED: ubsan-msan, ubsan-tsan
+
+#include <cassert>
+#include <cstddef>
+#include <cstdlib>
+
+void *operator new(std::size_t count) {
+  constexpr const size_t offset = 8;
+
+  // allocate a bit more so we can safely offset it
+  void *ptr = std::malloc(count + offset);
+
+  // verify malloc returned 16 bytes aligned mem
+  static_assert(__STDCPP_DEFAULT_NEW_ALIGNMENT__ == 16,
+                "Global new doesn't return 16 bytes aligned memory!");
+  assert((reinterpret_cast<std::ptrdiff_t>(ptr) &
+          (__STDCPP_DEFAULT_NEW_ALIGNMENT__ - 1)) == 0);
+
+  return static_cast<char *>(ptr) + offset;
+}
+
+struct Param {
+  void *_cookie1;
+  void *_cookie2;
+};
+
+static_assert(alignof(Param) == 8, "Param struct alignment must be 8 bytes!");
+
+int main() { Param *p = new Param; }

``````````

</details>


https://github.com/llvm/llvm-project/pull/152532