Skip to content

Commit

Permalink
Fix #25: Overload candidates overflow.
Browse files Browse the repository at this point in the history
This was actually two bugs:

First, the one mentioned in #25: invoke_normal() happily writes at array indices
which do not exist. Fixed by changing the array to std::vector. Required
ensuring proper destruction of invoke_context in case of lua_error.

Second, given enough candidates, the default Lua stack size of 20 elements would
not be enough to hold the error message parts before concatenation. Fixed by
concatenating after each candidate.
  • Loading branch information
Oberon00 committed Jan 28, 2015
1 parent efa9e51 commit ecfe25e
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 38 deletions.
16 changes: 8 additions & 8 deletions luabind/detail/call.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
# include <boost/type_traits/is_void.hpp>
# include <luabind/lua_include.hpp>

# include <vector>

namespace luabind { namespace detail {

struct invoke_context;
Expand Down Expand Up @@ -54,19 +56,17 @@ struct LUABIND_API invoke_context
{
invoke_context()
: best_score((std::numeric_limits<int>::max)())
, candidate_index(0)
{}

operator bool() const
{
return candidate_index == 1;
return candidates.size() == 1;
}

void format_error(lua_State* L, function_object const* overloads) const;

function_object const* candidates[10];
std::vector<function_object const*> candidates;
int best_score;
int candidate_index;
};

template <class F, class Signature, class Policies, class IsVoid>
Expand Down Expand Up @@ -264,12 +264,12 @@ invoke_normal
if (score >= 0 && score < ctx.best_score)
{
ctx.best_score = score;
ctx.candidates[0] = &self;
ctx.candidate_index = 1;
ctx.candidates.clear();
ctx.candidates.push_back(&self);
}
else if (score == ctx.best_score)
{
ctx.candidates[ctx.candidate_index++] = &self;
ctx.candidates.push_back(&self);
}

int results = 0;
Expand All @@ -279,7 +279,7 @@ invoke_normal
results = self.next->call(L, ctx);
}

if (score == ctx.best_score && ctx.candidate_index == 1)
if (score == ctx.best_score && ctx.candidates.size() == 1)
{
# ifndef LUABIND_INVOKE_VOID
result_converter.apply(
Expand Down
29 changes: 13 additions & 16 deletions luabind/make_function.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,36 +50,33 @@ namespace detail
*static_cast<function_object_impl const**>(
lua_touserdata(L, lua_upvalueindex(1)));

invoke_context ctx;

int results = 0;
bool error = false;

# ifndef LUABIND_NO_EXCEPTIONS
bool exception_caught = false;

try
#endif
// Scope neeeded to destroy invoke_context before calling lua_error()
{
invoke_context ctx;
results = invoke(
L, *impl, ctx, impl->f, Signature(), impl->policies);
if (!ctx)
{
ctx.format_error(L, impl);
error = true;
}
}
#ifndef LUABIND_NO_EXCEPTIONS
catch (...)
{
exception_caught = true;
error = true;
handle_exception_aux(L);
}
#endif

if (exception_caught)
lua_error(L);
# else
results = invoke(L, *impl, ctx, impl->f, Signature(), impl->policies);
# endif

if (!ctx)
{
ctx.format_error(L, impl);
if (error)
lua_error(L);
}

return results;
}

Expand Down
22 changes: 8 additions & 14 deletions src/function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,32 +104,26 @@ void invoke_context::format_error(
char const* function_name =
overloads->name.empty() ? "<unknown>" : overloads->name.c_str();

if (candidate_index == 0)
if (candidates.empty())
{
int stacksize = lua_gettop(L);
lua_pushliteral(L, "No matching overload found, candidates:\n");
int count = 0;
lua_pushliteral(L, "No matching overload found, candidates:");
for (function_object const* f = overloads; f != 0; f = f->next)
{
if (count != 0)
lua_pushliteral(L, "\n");
lua_pushliteral(L, "\n");
f->format_signature(L, function_name);
++count;
lua_concat(L, 3); // Inefficient, but does not use up the stack.
}
lua_concat(L, lua_gettop(L) - stacksize);
}
else
{
// Ambiguous
int stacksize = lua_gettop(L);
lua_pushliteral(L, "Ambiguous, candidates:\n");
for (int i = 0; i < candidate_index; ++i)
lua_pushliteral(L, "Ambiguous, candidates:");
for (std::size_t i = 0; i < candidates.size(); ++i)
{
if (i != 0)
lua_pushliteral(L, "\n");
lua_pushliteral(L, "\n");
candidates[i]->format_signature(L, function_name);
lua_concat(L, 3); // Inefficient, but does not use up the stack.
}
lua_concat(L, lua_gettop(L) - stacksize);
}
}

Expand Down
1 change: 1 addition & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ set(TESTS
function_converter
function_introspection
function_object
function_overload_overflow
held_type
implicit_cast
implicit_raw
Expand Down
46 changes: 46 additions & 0 deletions test/test_function_overload_overflow.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright Christian Neumüller 2015. Use, modification and distribution is
// subject to the Boost Software License, Version 1.0. (See accompanying
// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)

#include "test.hpp"
#include <luabind/function.hpp>

namespace {

void f() { }

} // namespace unnamed

void test_main(lua_State* L)
{
using namespace luabind;

module(L)
[ // 11 functions
def("f", &f), // 1
def("f", &f), // 2
def("f", &f), // 3
def("f", &f), // 4
def("f", &f), // 5
def("f", &f), // 6
def("f", &f), // 7
def("f", &f), // 8
def("f", &f), // 9
def("f", &f), // 10
def("f", &f) // 11
];

DOSTRING_EXPECTED(L, "f()",
"Ambiguous, candidates:\n" // 11 candidates
"void f()\n" // 1
"void f()\n" // 2
"void f()\n" // 3
"void f()\n" // 4
"void f()\n" // 5
"void f()\n" // 6
"void f()\n" // 7
"void f()\n" // 8
"void f()\n" // 9
"void f()\n" // 10
"void f()"); // 11
}

0 comments on commit ecfe25e

Please sign in to comment.