Skip to content

Commit

Permalink
Improve message on view out of bounds access and always abort (kokkos…
Browse files Browse the repository at this point in the history
…#6861)

* Add test for view out-of-bounds access

* [DO NOT MERGE] enable bound checking in view accesses unconditionally

* Abort on out-of-bounds access on the host side instead of throwing

* Improve out-of-bounds error message

* Add test for printing multidimensional index

* Try to get offsetview to compile

* Do not bother with the label on the device side

* Update test to reflect that device does not print the label

* Fixup OffsetView out-of-bounds on device

* Revert "Update test to reflect that device does not print the label"

This reverts commit 7dbecbf.

* Revert "Do not bother with the label on the device side"

This reverts commit 40348e8.

* Fixup UNMANAGED -> UNAVAILABLE

* Only enable the test when debug bound checking is enabled

* Revert "[DO NOT MERGE] enable bound checking in view accesses unconditionally"

This reverts commit ff9d411.

* Fix typo formated -> formatted

* Drop unecesary cast to void and Kokkos::Impl:: qualification

* Add test with mixed integer types

* Improve function name check_bounds -> within_range

* Prefer right fold per review

* Per review drop immediately invoked lambda trick on the host side

* Silent warnings about tracker variable not being used on the device
  • Loading branch information
dalg24 committed Mar 11, 2024
1 parent da77d6e commit a2b64e0
Show file tree
Hide file tree
Showing 4 changed files with 243 additions and 98 deletions.
11 changes: 2 additions & 9 deletions containers/src/Kokkos_OffsetView.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,15 +124,8 @@ KOKKOS_INLINE_FUNCTION void offsetview_verify_operator_bounds(
args...);
Kokkos::Impl::throw_runtime_exception(std::string(buffer));))

KOKKOS_IF_ON_DEVICE((
/* Check #1: is there a SharedAllocationRecord?
(we won't use it, but if it is not there then there isn't
a corresponding SharedAllocationHeader containing a label).
This check should cover the case of Views that don't
have the Unmanaged trait but were initialized by pointer. */
if (tracker.has_record()) {
Kokkos::Impl::operator_bounds_error_on_device(map);
} else { Kokkos::abort("OffsetView bounds error"); }))
KOKKOS_IF_ON_DEVICE(
(Kokkos::abort("OffsetView bounds error"); (void)tracker;))
}
}

Expand Down
154 changes: 65 additions & 89 deletions core/src/impl/Kokkos_ViewMapping.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3537,110 +3537,86 @@ class ViewMapping<
namespace Kokkos {
namespace Impl {

template <unsigned, class MapType>
KOKKOS_INLINE_FUNCTION bool view_verify_operator_bounds(const MapType&) {
return true;
template <class Map, class... Indices, std::size_t... Enumerate>
KOKKOS_FUNCTION bool within_range(Map const& map,
std::index_sequence<Enumerate...>,
Indices... indices) {
return (((std::size_t)indices < map.extent(Enumerate)) && ...);
}

template <unsigned R, class MapType, class iType, class... Args>
KOKKOS_INLINE_FUNCTION bool view_verify_operator_bounds(const MapType& map,
const iType& i,
Args... args) {
return (size_t(i) < map.extent(R)) &&
view_verify_operator_bounds<R + 1>(map, args...);
template <class... Indices>
KOKKOS_FUNCTION constexpr char* append_formatted_multidimensional_index(
char* dest, Indices... indices) {
char* d = dest;
strcat(d, "[");
(
[&] {
d += strlen(d);
to_chars_i(d,
d + 20, // 20 digits ought to be enough
indices);
strcat(d, ",");
}(),
...);
d[strlen(d) - 1] = ']'; // overwrite trailing comma
return dest;
}

template <unsigned, class MapType>
inline void view_error_operator_bounds(char*, int, const MapType&) {}

template <unsigned R, class MapType, class iType, class... Args>
inline void view_error_operator_bounds(char* buf, int len, const MapType& map,
const iType& i, Args... args) {
const int n = snprintf(
buf, len, " %ld < %ld %c", static_cast<unsigned long>(i),
static_cast<unsigned long>(map.extent(R)), (sizeof...(Args) ? ',' : ')'));
view_error_operator_bounds<R + 1>(buf + n, len - n, map, args...);
template <class Map, class... Indices, std::size_t... Enumerate>
KOKKOS_FUNCTION void print_extents(char* dest, Map const& map,
std::index_sequence<Enumerate...>) {
append_formatted_multidimensional_index(dest, map.extent(Enumerate)...);
}

/* Check #3: is the View managed as determined by the MemoryTraits? */
template <class MapType, bool is_managed = (MapType::is_managed != 0)>
struct OperatorBoundsErrorOnDevice;

template <class MapType>
struct OperatorBoundsErrorOnDevice<MapType, false> {
KOKKOS_INLINE_FUNCTION
static void run(MapType const&) { Kokkos::abort("View bounds error"); }
};

template <class MapType>
struct OperatorBoundsErrorOnDevice<MapType, true> {
KOKKOS_INLINE_FUNCTION
static void run(MapType const& map) {
SharedAllocationHeader const* const header =
SharedAllocationHeader::get_header(
static_cast<void const*>(map.data()));
char const* const label = header->label();
enum { LEN = 128 };
char msg[LEN];
char const* const first_part = "View bounds error of view ";
char* p = msg;
char* const end = msg + LEN - 1;
for (char const* p2 = first_part; (*p2 != '\0') && (p < end); ++p, ++p2) {
*p = *p2;
}
for (char const* p2 = label; (*p2 != '\0') && (p < end); ++p, ++p2) {
*p = *p2;
}
*p = '\0';
Kokkos::abort(msg);
}
};

/* Check #2: does the ViewMapping have the printable_label_typedef defined?
See above that only the non-specialized standard-layout ViewMapping has
this defined by default.
The existence of this alias indicates the existence of MapType::is_managed
*/
template <class T>
using printable_label_typedef_t = typename T::printable_label_typedef;

template <class Map>
KOKKOS_FUNCTION
std::enable_if_t<!is_detected<printable_label_typedef_t, Map>::value>
operator_bounds_error_on_device(Map const&) {
Kokkos::abort("View bounds error");
}

template <class Map>
KOKKOS_FUNCTION
std::enable_if_t<is_detected<printable_label_typedef_t, Map>::value>
operator_bounds_error_on_device(Map const& map) {
OperatorBoundsErrorOnDevice<Map>::run(map);
}

template <class MemorySpace, class ViewType, class MapType, class... Args>
KOKKOS_INLINE_FUNCTION void view_verify_operator_bounds(
Kokkos::Impl::ViewTracker<ViewType> const& tracker, const MapType& map,
Args... args) {
if (!view_verify_operator_bounds<0>(map, args...)) {
if (!within_range(map, std::make_index_sequence<sizeof...(Args)>(),
args...)) {
char err[256] = "";
strcat(err, "Kokkos::View ERROR: out of bounds access");
strcat(err, " label=(\"");
KOKKOS_IF_ON_HOST(
(enum {LEN = 1024}; char buffer[LEN];
const std::string label =
tracker.m_tracker.template get_label<MemorySpace>();
int n = snprintf(buffer, LEN, "View bounds error of view %s (",
label.c_str());
view_error_operator_bounds<0>(buffer + n, LEN - n, map, args...);
Kokkos::Impl::throw_runtime_exception(std::string(buffer));))

KOKKOS_IF_ON_DEVICE((
/* Check #1: is there a SharedAllocationRecord?
(we won't use it, but if its not there then there isn't
a corresponding SharedAllocationHeader containing a label).
This check should cover the case of Views that don't
have the Unmanaged trait but were initialized by pointer. */
if (tracker.m_tracker.has_record()) {
operator_bounds_error_on_device(map);
} else { Kokkos::abort("View bounds error"); }))
strncat(err, tracker.m_tracker.template get_label<void>().c_str(),
128);
} else { strcat(err, "**UNMANAGED**"); })
KOKKOS_IF_ON_DEVICE([&] {
// Check #1: is there a SharedAllocationRecord? (we won't use it, but
// if its not there then there isn't a corresponding
// SharedAllocationHeader containing a label). This check should cover
// the case of Views that don't have the Unmanaged trait but were
// initialized by pointer.
if (!tracker.m_tracker.has_record()) {
strcat(err, "**UNMANAGED**");
return;
}
// Check #2: does the ViewMapping have the printable_label_typedef
// defined? See above that only the non-specialized standard-layout
// ViewMapping has this defined by default. The existence of this
// alias indicates the existence of MapType::is_managed
if constexpr (is_detected_v<printable_label_typedef_t, MapType>) {
// Check #3: is the View managed as determined by the MemoryTraits?
if constexpr (MapType::is_managed != 0) {
SharedAllocationHeader const* const header =
SharedAllocationHeader::get_header(
static_cast<void const*>(map.data()));
char const* const label = header->label();
strcat(err, label);
return;
}
strcat(err, "**UNAVAILABLE**");
}
}();)
strcat(err, "\") with indices ");
append_formatted_multidimensional_index(err, args...);
strcat(err, " but extents ");
print_extents(err, map, std::make_index_sequence<sizeof...(Args)>());
Kokkos::abort(err);
}
}

Expand Down
1 change: 1 addition & 0 deletions core/unit_test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ foreach(Tag Threads;Serial;OpenMP;Cuda;HPX;OpenMPTarget;OpenACC;HIP;SYCL)
ViewMapping_subview
ViewMemoryAccessViolation
ViewOfClass
ViewOutOfBoundsAccess
ViewResize
WorkGraph
WithoutInitializing
Expand Down
175 changes: 175 additions & 0 deletions core/unit_test/TestViewOutOfBoundsAccess.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
//@HEADER
// ************************************************************************
//
// Kokkos v. 4.0
// Copyright (2022) National Technology & Engineering
// Solutions of Sandia, LLC (NTESS).
//
// Under the terms of Contract DE-NA0003525 with NTESS,
// the U.S. Government retains certain rights in this software.
//
// Part of Kokkos, under the Apache License v2.0 with LLVM Exceptions.
// See https://kokkos.org/LICENSE for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//@HEADER

#include <Kokkos_Core.hpp>
#include <sstream>

#include <gtest/gtest.h>

namespace {

TEST(TEST_CATEGORY, append_formatted_multidimensional_index) {
using Kokkos::Impl::append_formatted_multidimensional_index;
{
char buffer[64] = "my prefix ";
append_formatted_multidimensional_index(buffer, 1);
EXPECT_STREQ(buffer, "my prefix [1]");
}
{
char buffer[64] = "I was here";
append_formatted_multidimensional_index(buffer, 1, 2, 3);
EXPECT_STREQ(buffer, "I was here[1,2,3]");
}
{
char buffer[64] = "with mixed integer types ";
append_formatted_multidimensional_index(buffer, 1u, -2);
EXPECT_STREQ(buffer, "with mixed integer types [1,-2]");
}
}

#ifdef KOKKOS_ENABLE_DEBUG_BOUNDS_CHECK

template <class View, class ExecutionSpace>
struct TestViewOutOfBoundAccess {
View v;
static constexpr auto rank = View::rank;

template <std::size_t... Is>
KOKKOS_FUNCTION decltype(auto) bad_access(std::index_sequence<Is...>) const {
return v((Is * 1 + Is == 0 ? v.extent(Is) + 3 : 0)...);
}

KOKKOS_FUNCTION void operator()(int) const {
++bad_access(std::make_index_sequence<rank>{});
}

template <std::size_t... Is>
std::string get_details(std::index_sequence<Is...>) {
std::stringstream ss;
ss << "with indices \\[";
((ss << (Is == 0 ? v.extent(Is) + 3 : 0)
<< (Is == View::rank() - 1 ? "\\]" : ",")),
...);
ss << " but extents \\[";
((ss << v.extent(Is) << (Is == View::rank() - 1 ? "\\]" : ",")), ...);
return ss.str();
}

auto get_details() {
return get_details(std::make_index_sequence<View::rank()>());
}

TestViewOutOfBoundAccess(View w, ExecutionSpace const& s, std::string matcher)
: v(std::move(w)) {
constexpr bool view_accessible_from_execution_space =
Kokkos::SpaceAccessibility<
/*AccessSpace=*/ExecutionSpace,
/*MemorySpace=*/typename View::memory_space>::accessible;
EXPECT_TRUE(view_accessible_from_execution_space);

matcher += ".*" + get_details();

EXPECT_DEATH(
{
Kokkos::parallel_for(Kokkos::RangePolicy<ExecutionSpace>(s, 0, 1),
*this);
Kokkos::fence();
},
matcher);
}
};

template <class View, class LblOrPtr, std::size_t... Is>
auto make_view_impl(LblOrPtr x, std::index_sequence<Is...>) {
return View(x, (Is + 1)...);
}

template <class View, class LblOrPtr>
auto make_view(LblOrPtr x) {
return make_view_impl<View>(std::move(x),
std::make_index_sequence<View::rank>());
}

template <class ExecutionSpace>
void test_view_out_of_bounds_access() {
ExecutionSpace const exec_space{};
// clang-format off
using V1 = Kokkos::View<int*, ExecutionSpace>;
using V2 = Kokkos::View<int**, ExecutionSpace>;
using V3 = Kokkos::View<int***, ExecutionSpace>;
using V4 = Kokkos::View<int****, ExecutionSpace>;
using V5 = Kokkos::View<int*****, ExecutionSpace>;
using V6 = Kokkos::View<int******, ExecutionSpace>;
using V7 = Kokkos::View<int*******, ExecutionSpace>;
using V8 = Kokkos::View<int********, ExecutionSpace>;
std::string const prefix = "Kokkos::View ERROR: out of bounds access";
std::string const lbl = "my_label";
TestViewOutOfBoundAccess(make_view<V1>(lbl), exec_space, prefix + ".*" + lbl);
TestViewOutOfBoundAccess(make_view<V2>(lbl), exec_space, prefix + ".*" + lbl);
TestViewOutOfBoundAccess(make_view<V3>(lbl), exec_space, prefix + ".*" + lbl);
TestViewOutOfBoundAccess(make_view<V4>(lbl), exec_space, prefix + ".*" + lbl);
TestViewOutOfBoundAccess(make_view<V5>(lbl), exec_space, prefix + ".*" + lbl);
TestViewOutOfBoundAccess(make_view<V6>(lbl), exec_space, prefix + ".*" + lbl);
TestViewOutOfBoundAccess(make_view<V7>(lbl), exec_space, prefix + ".*" + lbl);
TestViewOutOfBoundAccess(make_view<V8>(lbl), exec_space, prefix + ".*" + lbl);
int* const ptr = nullptr;
TestViewOutOfBoundAccess(make_view<V1>(ptr), exec_space, prefix + ".*UNMANAGED");
TestViewOutOfBoundAccess(make_view<V2>(ptr), exec_space, prefix + ".*UNMANAGED");
TestViewOutOfBoundAccess(make_view<V3>(ptr), exec_space, prefix + ".*UNMANAGED");
TestViewOutOfBoundAccess(make_view<V4>(ptr), exec_space, prefix + ".*UNMANAGED");
TestViewOutOfBoundAccess(make_view<V5>(ptr), exec_space, prefix + ".*UNMANAGED");
TestViewOutOfBoundAccess(make_view<V6>(ptr), exec_space, prefix + ".*UNMANAGED");
TestViewOutOfBoundAccess(make_view<V7>(ptr), exec_space, prefix + ".*UNMANAGED");
TestViewOutOfBoundAccess(make_view<V8>(ptr), exec_space, prefix + ".*UNMANAGED");
// clang-format on
}

TEST(TEST_CATEGORY_DEATH, view_out_of_bounds_access) {
::testing::FLAGS_gtest_death_test_style = "threadsafe";

using ExecutionSpace = TEST_EXECSPACE;

if (false && Kokkos::SpaceAccessibility<
/*AccessSpace=*/ExecutionSpace,
/*MemorySpace=*/Kokkos::HostSpace>::accessible) {
GTEST_SKIP() << "skipping since no memory access violation would occur";
}

#if defined(KOKKOS_ENABLE_SYCL) && defined(NDEBUG) // FIXME_SYCL
if (std::is_same_v<ExecutionSpace, Kokkos::Experimental::SYCL>) {
GTEST_SKIP() << "skipping SYCL device-side abort does not work when NDEBUG "
"is defined";
}
#endif
#if defined(KOKKOS_ENABLE_OPENMPTARGET) // FIXME_OPENMPTARGET
if (std::is_same_v<ExecutionSpace, Kokkos::Experimental::OpenMPTarget>) {
GTEST_SKIP() << "skipping because OpenMPTarget backend is currently not "
"able to abort from the device";
}
#endif
#if defined(KOKKOS_ENABLE_OPENACC) // FIXME_OPENACC
if (std::is_same<ExecutionSpace, Kokkos::Experimental::OpenACC>::value) {
GTEST_SKIP() << "skipping because OpenACC backend is currently not "
"able to abort from the device";
}
#endif

test_view_out_of_bounds_access<ExecutionSpace>();
}

#endif

} // namespace

0 comments on commit a2b64e0

Please sign in to comment.