Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overloaded function calling wrong version, in release mode only #900

Closed
JoshKlint opened this issue Nov 20, 2019 · 13 comments
Closed

Overloaded function calling wrong version, in release mode only #900

JoshKlint opened this issue Nov 20, 2019 · 13 comments
Assignees
Milestone

Comments

@JoshKlint
Copy link

JoshKlint commented Nov 20, 2019

I have a problem with this overload:

L->set_function("CreateWidget",
	sol::overload(
		sol::resolve<shared_ptr<Widget>(shared_ptr<GUI>, const std::string&, const int, const int, const int, const int)>(&CreateWidget),
		sol::resolve<shared_ptr<Widget>(shared_ptr<Widget>, const std::string&, const int, const int, const int, const int)>(&CreateWidget)
	)
);

In debug mode it works as expected. In release mode, the first function is always called, even though the parameter calls for the second overload. This was quite difficult to track down. What is going on here?

Both the GUI and Widget class are derived from a base Object class.

@JoshKlint JoshKlint changed the title Overloaded function calling wrong version Overloaded function calling wrong version, in release mode only Nov 21, 2019
@ThePhD
Copy link
Owner

ThePhD commented Nov 21, 2019

Sounds like the safeties are getting turned off in Release mode and you're getting any-old-matching, because strict checking is disabled.

@ThePhD ThePhD self-assigned this Nov 21, 2019
@ThePhD ThePhD added this to the Helpdesk milestone Nov 21, 2019
@JoshKlint
Copy link
Author

JoshKlint commented Nov 21, 2019

How do I change that? The function this is being called from within is being executed using lua_pcall().

@JoshKlint
Copy link
Author

JoshKlint commented Nov 21, 2019

It looks like this code has an example of the sol way to replace lua_pcall, and from what I gather, the sol calling methods add the safety checks in(?):
#865

Do I need to not use lua_pcall? Is there another replacement for lua_pcall?

@JoshKlint
Copy link
Author

I replaced the call to lua_pcall with this code but the problem persists!

bool Entity::CallFunction(const std::string& name)
{
	if (VirtualMachine::L == nullptr) VirtualMachine::Reset();

	auto L = VirtualMachine::L;
	bool success = false;
	auto self = dynamic_pointer_cast<Entity>(Self());
		 
	int stacksize = lua_gettop(L);
	sol::stack::push(L, self);
	sol::stack::get_field(L, name);
	if (lua_isfunction(L, -1))
	{
		auto f = sol::protected_function(L, -1);
		if (f.valid())
		{
			auto result = f(self);
			if (result.status() == sol::call_status::ok) success = true;
		}
	}

	lua_settop(L, stacksize);
	return success;
}

@JoshKlint
Copy link
Author

JoshKlint commented Nov 21, 2019

I defined SOL_ALL_SAFETIES_ON = 1 and an error is occurring when the above code is run, but I don't know how to catch the error and print it out.

auto f = sol::protected_function(L);	
if (f.valid())
{
	sol::protected_function_result result;
	try
	{
		result = f(self);
	}
	catch (const sol::error & e)
	{
		Print("Lua error: " + std::string(e.what()));
	}
	if (result.status() == sol::call_status::ok)
	{					
		success = true;
	}
	else
	{
		Print("Some error occurred, I don't know what!");
	}
}

@JoshKlint
Copy link
Author

Okay, I have error handling working now. It says that no overload of the function was found with the given arguments, but I know it is there.

auto L = VirtualMachine::L;
bool success = false;
auto self = dynamic_pointer_cast<Entity>(Self());
		
sol::function errorfunc = {};
lua_getglobal(L, "LuaErrorHandler");
if (lua_isfunction(L, -1)) errorfunc = sol::function(L);

int stacksize = lua_gettop(L);
sol::stack::push(L, self);
sol::stack::get_field(L, name);
if (lua_isfunction(L, -1))
{
	auto f = sol::protected_function(L);	
	if (f.valid())
	{
		if (errorfunc.valid()) f.set_default_handler(errorfunc);
		sol::protected_function_result result;
		try
		{
			result = f(self);
		}
		catch (const sol::error & e)
		{
			Print("Lua error: " + std::string(e.what()));
		}
		if (result.status() == sol::call_status::ok)
		{					
			success = true;
		}
		else
		{
			Print("Some error occurred, I don't know what lol!");
		}
	}
}

@JoshKlint
Copy link
Author

JoshKlint commented Nov 21, 2019

I found one way to write the function that works correctly in release mode:

L->set_function("CreateWidget",
	sol::overload
	(
		[](Widget* wid, std::string s, int x, int y, int w, int h) { return CreateWidget(dynamic_pointer_cast<Widget>(wid->Self()), s, x, y, w, h); },
		//sol::resolve<shared_ptr<Widget>(shared_ptr<Widget>, const std::string&, const int, const int, const int, const int)>(&CreateWidget),
		sol::resolve<shared_ptr<Widget>(shared_ptr<GUI>, const std::string&, const int, const int, const int, const int)>(&CreateWidget)
	)
);

I am using strict checks for everything except floating points / integers:

#define SOL_NO_CHECK_NUMBER_PRECISION 1
#define SOL_ALL_SAFETIES_ON 1
#include <sol/sol.hpp>

It is pretty dangerous to have these types of errors occurring. Any ideas how this can be detected better?

@ThePhD
Copy link
Owner

ThePhD commented Nov 24, 2019

After deep investigation, this seems like a SERIOUS bug in Visual C++. In release builds, it is selecting the wrong template function and this is extremely concerning to me that it is doing this despite there not being an ODR violation that I can find.

I will need to spend more time looking into this and possibly producing a repro for the VC++ team, or finding out if there is some deeper bug in my code. I can't guarantee I will get back to you quickly, as I have spent a lot of my time just to confirm Visual C++ might be behaving poorly...

@stonedreamforest
Copy link

@ThePhD
I have encountered this problem before ,try disable comdat folding, this is a vc++ bug, Because it can't be reproduced in other compilers.

@ThePhD
Copy link
Owner

ThePhD commented Nov 25, 2019

Well, the docs there make it easy to see why RelWithDebInfo builds don't show the problem and only strictly release builds...

Goddamn I spent days tracking this. Thanks for the tip @stonedreamforest

@ThePhD
Copy link
Owner

ThePhD commented Nov 25, 2019

Specifying /OPT:NOREF,NOICF,NOLBR to the linker makes this program work properly.

This is a Visual C++ bug. Nothing I can do to help you here, @JoshKlint ; either write the function different or turn it off.

#include <iostream>

#define SOL_ALL_SAFETIES_ON 1
#include <sol/sol.hpp>

struct Object {
	int value;
};

struct Widget : Object {
	double wid_value;

	Widget(int v, double wv) {
		this->value = v;
		this->wid_value = wv;
	}
};

struct Wadget : Object {
	bool wad_value;

	Wadget(int v, bool wv) {
		this->value = v;
		this->wad_value = wv;
	}
};

std::shared_ptr<Object> f (std::shared_ptr<Widget>, int a) {
	std::cout << "Widget" << std::endl;
	return std::make_shared<Widget>(a, 4.0);
}

std::shared_ptr<Object> g (std::shared_ptr<Wadget>, int a) {
	std::cout << "Wadget" << std::endl;
	return std::make_shared<Wadget>(a, false);
}

int main() {
	sol::state lua;
	lua.open_libraries(sol::lib::base);

	//lua.new_usertype<Widget>("Widget", sol::base_classes, sol::bases<Object>());
	//lua.new_usertype<Wadget>("Wadget", sol::base_classes, sol::bases<Object>());

	std::shared_ptr<Widget> wid = std::make_shared<Widget>(0, 2.0);
	std::shared_ptr<Wadget> wad = std::make_shared<Wadget>(1, true);

	lua["create"] = sol::overload(&f, &g);

	sol::protected_function create = lua["create"];
	std::cout << "create(wid, 2)" << std::endl;
	create(wid, 2);
	std::cout << "create(wad, 4)" << std::endl;
	create(wad, 4);

	return 0;
}

@ThePhD ThePhD closed this as completed Nov 25, 2019
@JoshKlint
Copy link
Author

Wow, that is interesting. I also lost a couple days on this trying to figure out what was going on. My program works now with the original commands, with the fix above. Thank you!

@romanholidaypancakes
Copy link

Specifying /OPT:NOREF,NOICF,NOLBR to the linker makes this program work properly.

This is a Visual C++ bug. Nothing I can do to help you here, @JoshKlint ; either write the function different or turn it off.

#include <iostream>

#define SOL_ALL_SAFETIES_ON 1
#include <sol/sol.hpp>

struct Object {
	int value;
};

struct Widget : Object {
	double wid_value;

	Widget(int v, double wv) {
		this->value = v;
		this->wid_value = wv;
	}
};

struct Wadget : Object {
	bool wad_value;

	Wadget(int v, bool wv) {
		this->value = v;
		this->wad_value = wv;
	}
};

std::shared_ptr<Object> f (std::shared_ptr<Widget>, int a) {
	std::cout << "Widget" << std::endl;
	return std::make_shared<Widget>(a, 4.0);
}

std::shared_ptr<Object> g (std::shared_ptr<Wadget>, int a) {
	std::cout << "Wadget" << std::endl;
	return std::make_shared<Wadget>(a, false);
}

int main() {
	sol::state lua;
	lua.open_libraries(sol::lib::base);

	//lua.new_usertype<Widget>("Widget", sol::base_classes, sol::bases<Object>());
	//lua.new_usertype<Wadget>("Wadget", sol::base_classes, sol::bases<Object>());

	std::shared_ptr<Widget> wid = std::make_shared<Widget>(0, 2.0);
	std::shared_ptr<Wadget> wad = std::make_shared<Wadget>(1, true);

	lua["create"] = sol::overload(&f, &g);

	sol::protected_function create = lua["create"];
	std::cout << "create(wid, 2)" << std::endl;
	create(wid, 2);
	std::cout << "create(wad, 4)" << std::endl;
	create(wad, 4);

	return 0;
}

hi @ThePhD
Is this still a problem in vs2022? I don't want to close it because closing makes the file bigger.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants