[PATCH] D116861: [UBSan] Fix incorrect alignment reported when global new returns an offset pointer

[Lambert Clara via Phabricator via cfe-commits]

belkiss updated this revision to Diff 399764.
belkiss added a comment.

Simplify test, remove the inline specific CHECK-NOT since we have the 
implicit-check-not in FileCheck args checking that no errors have occured in 
the whole file


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116861/new/

https://reviews.llvm.org/D116861

Files:
  clang/lib/CodeGen/CGExprCXX.cpp
  compiler-rt/test/ubsan/TestCases/TypeCheck/global-new-alignment.cpp


Index: compiler-rt/test/ubsan/TestCases/TypeCheck/global-new-alignment.cpp
===================================================================
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/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; }
Index: clang/lib/CodeGen/CGExprCXX.cpp
===================================================================
--- clang/lib/CodeGen/CGExprCXX.cpp
+++ clang/lib/CodeGen/CGExprCXX.cpp
@@ -1731,8 +1731,8 @@
   SkippedChecks.set(SanitizerKind::Null, nullCheck);
   EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall,
                 E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
-                result.getPointer(), allocType, result.getAlignment(),
-                SkippedChecks, numElements);
+                result.getPointer(), allocType, allocAlign, SkippedChecks,
+                numElements);
 
   EmitNewInitializer(*this, E, allocType, elementTy, result, numElements,
                      allocSizeWithoutCookie);



Index: compiler-rt/test/ubsan/TestCases/TypeCheck/global-new-alignment.cpp
===================================================================
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/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; }
Index: clang/lib/CodeGen/CGExprCXX.cpp
===================================================================
--- clang/lib/CodeGen/CGExprCXX.cpp
+++ clang/lib/CodeGen/CGExprCXX.cpp
@@ -1731,8 +1731,8 @@
   SkippedChecks.set(SanitizerKind::Null, nullCheck);
   EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall,
                 E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
-                result.getPointer(), allocType, result.getAlignment(),
-                SkippedChecks, numElements);
+                result.getPointer(), allocType, allocAlign, SkippedChecks,
+                numElements);
 
   EmitNewInitializer(*this, E, allocType, elementTy, result, numElements,
                      allocSizeWithoutCookie);

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits