Const reference returns are pushed as values, resulting in double deletion of memory

[erikogenvik]

If a C++ function returns a "const reference" object, that object will be pushed into Lua as a "value type", which means that the destructor of the reference will be called when Lua does garbage collection. This will lead to a double delete situation, with subsequent segfault.

Code that shows this:

	struct Foo {
		bool allow_destruction = false;
		~Foo() {
			REQUIRE(allow_destruction);
		}
	};

	Foo foo;

	sol::state lua;
	lua.new_usertype<Foo>("Foo");
	lua["getFooPtr"] = [&](){ return &foo;};
	lua["getFooRef"] = [&]() -> Foo& { return foo;};
	lua["getFooRefConst"] = [&]() -> const Foo& { return foo;};
	lua.safe_script("getFooPtr()");
	//Should not collect anything.
	lua.collect_gc();
	lua.safe_script("getFooRef()");
	//Should not collect anything.
	lua.collect_gc();
	lua.safe_script("getFooRefConst()");
	//Should not collect anything. But does!
	lua.collect_gc();
	foo.allow_destruction = true;

This is on OpenSuSE Tumbleweed 20220528, using gcc 12.1.0.

I've just looked through the code quickly, but this is my understanding so far:
stack_detail::push_reference performs a compile time check where it looks for references by looking at both std::is_lvalue_reference<T> as well as meta::neg<std::is_const<std::remove_reference_t<T>>>. I.e. if it's a "const ref" it's handled as a value type.
This check was added in commit 706ca80 back in 2016. However, the initial version didn't properly remove the reference before checking, which lead to the check always being negative, even for const references.
In 2020, with commit 0e1aafe, this was "fixed", by adding the std::remove_reference_t part. However, by doing this the new bug, where "const references" are deleted by Lua GC, was introduced.

It's not entirely clear to me why the initial check for const-ness was added here; the commit message doesn't really tell. Perhaps meta::unqualified_t behaved differently at that time and could handle references?

One solution is to just remove the const-ness check in push_reference. This is done in this pull request: #1364