[llvm-bugs] [Bug 48993] New: Recent shared_ptr storage change (https://reviews.llvm.org/D91201) causes CFI cast failures during make_shared

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=48993

            Bug ID: 48993
           Summary: Recent shared_ptr storage change
                    (https://reviews.llvm.org/D91201) causes CFI cast
                    failures during make_shared
           Product: libc++
           Version: unspecified
          Hardware: PC
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P
         Component: All Bugs
          Assignee: unassignedclangbugs at nondot.org
          Reporter: rnk at google.com
                CC: llvm-bugs at lists.llvm.org, mclow.lists at gmail.com

Consider this program that creates and destroys a shared_ptr object:

$ cat t.cpp 
#include <memory>
struct Foo {
  Foo() {}
  virtual ~Foo() {}
};
int main(int argc, char **argv) { std::make_shared<Foo>(); }

If you compile libc++, lld, clang, cfi, ubsan, etc, at ToT and build this
program like so, it results in CFI cast errors:

$ ./bin/clang++  t.cpp  -flto=thin -fuse-ld=lld -stdlib=libc++  -fsanitize=cfi
-fno-sanitize-trap  -fvisibility=hidden && LD_LIBRARY_PATH=lib  ./a.out   ;
echo $?
/usr/local/google/home/rnk/llvm-project/build/bin/../include/c++/v1/memory:2653:27:
runtime error: control flow integrity check for type 'Foo' failed during cast
to unrelated type (vtable address 0x000000000000)
0x000000000000: note: invalid vtable
<memory cannot be printed>
/usr/local/google/home/rnk/llvm-project/build/bin/../include/c++/v1/memory:2653:27:
note: check failed in /usr/local/google/home/rnk/llvm-project/build/a.out,
vtable located in (unknown)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
/usr/local/google/home/rnk/llvm-project/build/bin/../include/c++/v1/memory:2653:27
in 
1

Reverting 955dd7b7f3f6d / https://reviews.llvm.org/D91201 locally and
rebuilding libc++ to copy the headers makes the problem go away:

$ git revert 955dd7b7f3f6d
Removing
libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/allocate_shared_construct.pass.cpp
Removing
libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/libcxx.control_block_layout.pass.cpp
Auto-merging libcxx/include/memory
[main 5f7ab320457d] Revert "[libc++] LWG2070: Use Allocator construction for
objects created with allocate_shared"
 4 files changed, 19 insertions(+), 429 deletions(-)
 delete mode 100644
libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/libcxx.control_block_layout.pass.cpp
 delete mode 100644
libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/allocate_shared_construct.pass.cpp

$ ninja cxx
[1 processes, 11/11 @ 17.6/s : 0.626s ] Creating library symlink
lib/libc++.so.1 lib/libc++.so

$ ./bin/clang++  t.cpp  -flto=thin -fuse-ld=lld -stdlib=libc++  -fsanitize=cfi
-fno-sanitize-trap  -fvisibility=hidden && LD_LIBRARY_PATH=lib  ./a.out   ;
echo $?
0


These are the lines of the change that matter:

     _LIBCPP_HIDE_FROM_ABI
     explicit __shared_ptr_emplace(_Alloc __a, _Args&& ...__args)
-#ifndef _LIBCPP_CXX03_LANG
-        : __data_(piecewise_construct, _VSTD::forward_as_tuple(__a),
-                  _VSTD::forward_as_tuple(_VSTD::forward<_Args>(__args)...))
+        : __storage_(_VSTD::move(__a))
+    {
+#if _LIBCPP_STD_VER > 17
+        using _TpAlloc = typename __allocator_traits_rebind<_Alloc,
_Tp>::type;
+        _TpAlloc __tmp(*__get_alloc());
+        allocator_traits<_TpAlloc>::construct(__tmp, __get_elem(),
_VSTD::forward<_Args>(__args)...);
 #else
-        : __data_(__a, _Tp(_VSTD::forward<_Args>(__args)...))
+        ::new ((void*)__get_elem()) _Tp(_VSTD::forward<_Args>(__args)...);
 #endif

That change switches from calling the __compressed_pair constructor directly to
using placement new on the result of __get_elem. Inside __get_elem, the address
of the second pair element is cast to the type that will be constructed. That
reinterpret_cast on the memory before construction is the issue: CFI detects
cast confusion bugs by checking that the vtable is correct for the type of the
cast, but the vptr slot is still uninitialized.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20210201/183b6430/attachment.html>