Skip to content

Commit

Permalink
Add integer div-by-zero check
Browse files Browse the repository at this point in the history
Closes hsutter#1184

Only when the numerator and denominator are both integral types

Use -no-div-zero-checks to disable

Includes documentation
  • Loading branch information
hsutter committed Jul 30, 2024
1 parent daf9eec commit 63d02e8
Show file tree
Hide file tree
Showing 12 changed files with 118 additions and 28 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,4 @@ venv/*
.vscode/
buildh2.bat
gen_version.bat
mkdocs_serve.sh
6 changes: 5 additions & 1 deletion docs/cppfront/options.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,16 @@ This option also sets `-import-std`.
Print version, build, copyright, and license information.


## Additional dynamic safety checks and contract information
## Additional dynamic safety check controls

### `-no-comparison-checks`, `-no-c`

Disable mixed-sign comparison safety checks. If not disabled, mixed-sign comparisons are diagnosed by default.

### `-no-div-zero-checks`, `-no-d`

Disable integer division by zero checks. If not disabled, integer division by zero checks are performed by default.

### `-no-null-checks`, `-no-n`

Disable null safety checks. If not disabled, null dereference checks are performed by default.
Expand Down
61 changes: 55 additions & 6 deletions include/cpp2util.h
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ namespace impl {

//-----------------------------------------------------------------------
//
// Check for invalid dereference or indirection which would result in undefined behavior.
// Invalid/null dereference checking - cases that would result in UB.
//
// - Null pointer
// - std::unique_ptr that owns nothing
Expand Down Expand Up @@ -861,11 +861,60 @@ auto assert_not_null(auto&& arg CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> decl
return CPP2_FORWARD(arg);
}

// Subscript bounds checking

//-----------------------------------------------------------------------
//
// Integer divide-by-zero checking - cases that would result in UB.
//
// Notes:
// NumType is the Numerator type
// arg is the denominator value
// Both must be integral to enable the check
//

#define CPP2_ASSERT_NOT_ZERO_IMPL \
requires (std::is_integral_v<CPP2_TYPEOF(arg)> && \
std::is_integral_v<NumType>) \
{ \
if (0 == arg) { \
type_safety.report_violation("integer division by zero attempt detected" CPP2_SOURCE_LOCATION_ARG); \
} \
return arg; \
}

template<typename NumType, auto arg>
auto assert_not_zero([[maybe_unused]] char _ CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> auto
CPP2_ASSERT_NOT_ZERO_IMPL

template<typename NumType, auto arg>
auto assert_not_zero([[maybe_unused]] char _ CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> auto
{
return arg;
}

template<typename NumType>
auto assert_not_zero(auto arg CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> auto
CPP2_ASSERT_NOT_ZERO_IMPL

template<typename NumType>
auto assert_not_zero(auto&& arg CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> decltype(auto)
requires (!std::is_integral_v<CPP2_TYPEOF(arg)>
|| !std::is_integral_v<NumType>)
{
return CPP2_FORWARD(arg);
}

#define CPP2_ASSERT_NOT_ZERO(NumType, arg) (cpp2::impl::assert_not_zero<NumType>((arg)))
#define CPP2_ASSERT_NOT_ZERO_LITERAL(NumType, arg) (cpp2::impl::assert_not_zero<NumType, (arg)>('_'))


//-----------------------------------------------------------------------
//
// Subscript bounds checking - cases that would result in UB.
//
#define CPP2_ASSERT_IN_BOUNDS_IMPL \
requires (std::is_integral_v<CPP2_TYPEOF(arg)> && \
requires { std::size(x); std::ssize(x); x[arg]; std::begin(x) + 2; }) \
requires { std::size(x); std::ssize(x); x[arg]; std::begin(x) + 2; }) \
{ \
auto max = [&]() -> auto { \
if constexpr (std::is_signed_v<CPP2_TYPEOF(arg)>) { return std::ssize(x); } \
Expand All @@ -888,15 +937,15 @@ template<auto arg>
auto assert_in_bounds(auto&& x CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> decltype(auto)
CPP2_ASSERT_IN_BOUNDS_IMPL

auto assert_in_bounds(auto&& x, auto&& arg CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> decltype(auto)
CPP2_ASSERT_IN_BOUNDS_IMPL

template<auto arg>
auto assert_in_bounds(auto&& x CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> decltype(auto)
{
return CPP2_FORWARD(x) [ arg ];
}

auto assert_in_bounds(auto&& x, auto&& arg CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> decltype(auto)
CPP2_ASSERT_IN_BOUNDS_IMPL

auto assert_in_bounds(auto&& x, auto&& arg CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> decltype(auto)
{
return CPP2_FORWARD(x) [ CPP2_FORWARD(arg) ];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2:3:46: error: expect
In file included from pure2-bugfix-for-requires-clause-in-forward-declaration.cpp:7:
../../../include/cpp2util.h:10005:47: error: static assertion failed: GCC 11 or higher is required to support variables and type-scope functions that have a 'requires' clause. This includes a type-scope 'forward' parameter of non-wildcard type, such as 'func: (this, forward s: std::string)', which relies on being able to add a 'requires' clause - in that case, use 'forward s: _' instead if you need the result to compile with GCC 10.
pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2:4:1: note: in expansion of macro ‘CPP2_REQUIRES_’
pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2:3:3: error: no declaration matches ‘element::element(auto:259&&) requires is_same_v<std::__cxx11::string, typename std::remove_cv<typename std::remove_reference<decltype(element::__ct ::n)>::type>::type>’
pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2:3:3: error: no declaration matches ‘element::element(auto:261&&) requires is_same_v<std::__cxx11::string, typename std::remove_cv<typename std::remove_reference<decltype(element::__ct ::n)>::type>::type>’
pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2:5:11: note: candidates are: ‘element::element(const element&)’
pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2:3:20: note: ‘template<class auto:257> element::element(auto:257&&)’
pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2:3:20: note: ‘template<class auto:259> element::element(auto:259&&)’
pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2:1:7: note: ‘class element’ defined here
pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2:5:78: error: expected unqualified-id before ‘{’ token
pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2:3:8: error: no declaration matches ‘element& element::operator=(auto:260&&) requires is_same_v<std::__cxx11::string, typename std::remove_cv<typename std::remove_reference<decltype(element::operator=::n)>::type>::type>’
pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2:3:8: error: no declaration matches ‘element& element::operator=(auto:262&&) requires is_same_v<std::__cxx11::string, typename std::remove_cv<typename std::remove_reference<decltype(element::operator=::n)>::type>::type>’
pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2:6:16: note: candidates are: ‘void element::operator=(const element&)’
pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2:3:16: note: ‘template<class auto:258> element& element::operator=(auto:258&&)’
pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2:3:16: note: ‘template<class auto:260> element& element::operator=(auto:260&&)’
pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2:1:7: note: ‘class element’ defined here
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ pure2-print.cpp2:68:1: note: in expansion of macro ‘CPP2_REQUIRES_’
pure2-print.cpp2:97:1: note: in expansion of macro ‘CPP2_REQUIRES_’
pure2-print.cpp2:9:41: error: ‘constexpr const T outer::object_alias’ is not a static data member of ‘class outer’
pure2-print.cpp2:9:48: error: template definition of non-template ‘constexpr const T outer::object_alias’
pure2-print.cpp2:67:14: error: no declaration matches ‘void outer::mytype::variadic(const auto:258& ...) requires (is_convertible_v<typename std::remove_cv<typename std::remove_reference<decltype(outer::mytype::variadic::x)>::type>::type, int> && ...)’
pure2-print.cpp2:67:29: note: candidate is: ‘template<class ... auto:257> static void outer::mytype::variadic(const auto:257& ...)’
pure2-print.cpp2:67:14: error: no declaration matches ‘void outer::mytype::variadic(const auto:260& ...) requires (is_convertible_v<typename std::remove_cv<typename std::remove_reference<decltype(outer::mytype::variadic::x)>::type>::type, int> && ...)’
pure2-print.cpp2:67:29: note: candidate is: ‘template<class ... auto:259> static void outer::mytype::variadic(const auto:259& ...)’
pure2-print.cpp2:10:19: note: ‘class outer::mytype’ defined here
pure2-print.cpp2:96:37: error: no declaration matches ‘void outer::print(std::ostream&, const Args& ...) requires cpp2::impl::cmp_greater_eq(sizeof ... (Args ...), 0)’
pure2-print.cpp2:96:37: note: no functions named ‘void outer::print(std::ostream&, const Args& ...) requires cpp2::impl::cmp_greater_eq(sizeof ... (Args ...), 0)’
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
mixed-bounds-safety-with-assert.cpp2(11) void print_subrange(const auto:255&, cpp2::impl::in<int>, cpp2::impl::in<int>) [with auto:255 = std::vector<int>; cpp2::impl::in<int> = const int]: Bounds safety violation
mixed-bounds-safety-with-assert.cpp2(11) void print_subrange(const auto:257&, cpp2::impl::in<int>, cpp2::impl::in<int>) [with auto:257 = std::vector<int>; cpp2::impl::in<int> = const int]: Bounds safety violation
Original file line number Diff line number Diff line change
@@ -1,41 +1,41 @@
In file included from mixed-bugfix-for-ufcs-non-local.cpp:6:
../../../include/cpp2util.h:2100:1: error: lambda-expression in template parameter type
2100 |
2100 | constexpr auto is( std::optional<U> const& x ) -> bool
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 | ~finally() noexcept { f(); }
2137 | //
| ^
mixed-bugfix-for-ufcs-non-local.cpp2:13:12: note: in expansion of macro ‘CPP2_UFCS_NONLOCAL’
mixed-bugfix-for-ufcs-non-local.cpp2:13:36: error: template argument 1 is invalid
../../../include/cpp2util.h:2100:1: error: lambda-expression in template parameter type
2100 |
2100 | constexpr auto is( std::optional<U> const& x ) -> bool
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 | ~finally() noexcept { f(); }
2137 | //
| ^
mixed-bugfix-for-ufcs-non-local.cpp2:21:12: note: in expansion of macro ‘CPP2_UFCS_NONLOCAL’
mixed-bugfix-for-ufcs-non-local.cpp2:21:36: error: template argument 1 is invalid
../../../include/cpp2util.h:2100:1: error: lambda-expression in template parameter type
2100 |
2100 | constexpr auto is( std::optional<U> const& x ) -> bool
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 | ~finally() noexcept { f(); }
2137 | //
| ^
mixed-bugfix-for-ufcs-non-local.cpp2:31:12: note: in expansion of macro ‘CPP2_UFCS_NONLOCAL’
mixed-bugfix-for-ufcs-non-local.cpp2:31:36: error: template argument 1 is invalid
../../../include/cpp2util.h:2100:1: error: lambda-expression in template parameter type
2100 |
2100 | constexpr auto is( std::optional<U> const& x ) -> bool
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 | ~finally() noexcept { f(); }
2137 | //
| ^
mixed-bugfix-for-ufcs-non-local.cpp2:33:12: note: in expansion of macro ‘CPP2_UFCS_NONLOCAL’
mixed-bugfix-for-ufcs-non-local.cpp2:33:36: error: template argument 1 is invalid
../../../include/cpp2util.h:2100:1: error: lambda-expression in template parameter type
2100 |
2100 | constexpr auto is( std::optional<U> const& x ) -> bool
| ^
../../../include/cpp2util.h:2137:59: note: in expansion of macro ‘CPP2_UFCS_’
2137 | ~finally() noexcept { f(); }
2137 | //
| ^
mixed-bugfix-for-ufcs-non-local.cpp2:21:12: note: in expansion of macro ‘CPP2_UFCS_NONLOCAL’
mixed-bugfix-for-ufcs-non-local.cpp2:21:36: error: template argument 1 is invalid
2 changes: 1 addition & 1 deletion regression-tests/test-results/pure2-print.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ requires (true) inline CPP2_CONSTEXPR T outer::object_alias{ 42 };
if (cpp2::impl::cmp_less(*cpp2::impl::assert_not_null(p),0)) {
ret = -*cpp2::impl::assert_not_null(cpp2::move(p));
}
ret += strlen(s) - 10 + CPP2_UFCS(strlen)(s) * (16 / (3 & 2)) % 3;
ret += strlen(s) - 10 + CPP2_UFCS(strlen)(s) * (16 / CPP2_ASSERT_NOT_ZERO(CPP2_TYPEOF(16),(3 & 2))) % 3;

map<int const,string> m {};
CPP2_ASSERT_IN_BOUNDS_LITERAL(m, 0) = cpp2::impl::as_<string>("har");
Expand Down
2 changes: 1 addition & 1 deletion regression-tests/test-results/version
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

cppfront compiler v0.7.2 Build 9727:1056
cppfront compiler v0.7.2 Build 9729:1513
Copyright(c) Herb Sutter All rights reserved

SPDX-License-Identifier: CC-BY-NC-ND-4.0
Expand Down
2 changes: 1 addition & 1 deletion source/build.info
Original file line number Diff line number Diff line change
@@ -1 +1 @@
"9727:1056"
"9729:1513"
2 changes: 1 addition & 1 deletion source/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,7 @@ class cmdline_processor
int max_flag_length = 0;

std::unordered_map<int, std::string> labels = {
{ 2, "Additional dynamic safety checks and contract information" },
{ 2, "Additional dynamic safety check controls" },
{ 4, "Support for constrained target environments" },
{ 8, "Cpp1 file content options" },
{ 9, "Cppfront output options" }
Expand Down
36 changes: 36 additions & 0 deletions source/to_cpp1.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,14 @@ static cmdline_processor::register_flag cmd_safe_null_pointers(
[]{ flag_safe_null_pointers = false; }
);

static auto flag_safe_zero_division = true;
static cmdline_processor::register_flag cmd_safe_zero_division(
2,
"no-div-zero-checks",
"Disable integer division by zero checks",
[]{ flag_safe_zero_division = false; }
);

static auto flag_safe_subscripts = true;
static cmdline_processor::register_flag cmd_safe_subscripts(
2,
Expand Down Expand Up @@ -3868,6 +3876,7 @@ class cppfront
}
// If it's "_ =" then emit static_cast<void>()
bool emit_discard = false;
auto last_expr = decltype(n.expr.get()){};
if (
!n.terms.empty()
&& n.terms.front().op->type() == lexeme::Assignment
Expand All @@ -3881,6 +3890,7 @@ class cppfront
}
else
{
last_expr = n.expr.get();
emit(*n.expr);
}
suppress_move_from_last_use = false;
Expand Down Expand Up @@ -3972,7 +3982,33 @@ class cppfront
}
// Otherwise, just emit the general expression as usual
else {
// If this is a division, wrap the denominator in a not-zero check
auto suffix = std::string{};
if (
flag_safe_zero_division
&& (
x.op->type() == lexeme::Slash
|| x.op->type() == lexeme::SlashEq
)
)
{
assert(last_expr && "ICE: shouldn't get here without having captured a pointer to the numerator expression");
if (auto lit = x.expr->get_literal();
lit
&& lit->get_token()->type() == lexeme::DecimalLiteral
)
{
printer.print_cpp2( "CPP2_ASSERT_NOT_ZERO_LITERAL(CPP2_TYPEOF(" + print_to_string(*last_expr) + "),", x.op->position() );
}
else
{
printer.print_cpp2( "CPP2_ASSERT_NOT_ZERO(CPP2_TYPEOF(" + print_to_string(*last_expr) + "),", x.op->position() );
}
suffix = ")";
}
last_expr = x.expr.get();
emit(*x.expr);
printer.print_cpp2( suffix, x.expr->position() );
}
}

Expand Down

0 comments on commit 63d02e8

Please sign in to comment.