Fixed undefined behaviour when love exceptions are converted into Lua errors (resolves issue #729)

src/modules/filesystem/physfs/Filesystem.cpp

@@ -529,15 +529,19 @@ int Filesystem::lines_i(lua_State *L)
 		}
 		else
 		{
-			char *str;
+			char *str = 0;
 			try
 			{
 				str = new char[linesize + 1];
 			}
 			catch(std::bad_alloc &)
 			{
-				return luaL_error(L, "Out of memory");
+				// Can't lua_error (longjmp) in exception handlers.
 			}
+
+			if (!str)
+				return luaL_error(L, "Out of memory.");
+
 			file->seek(pos);
 
 			// Read the \n anyway and save us a call to seek.

src/modules/filesystem/physfs/wrap_File.cpp

@@ -24,7 +24,14 @@
 #include "common/Exception.h"
 #include "common/int.h"
 
-static int ioError(lua_State *L, const char *fmt, ...)
+namespace love
+{
+namespace filesystem
+{
+namespace physfs
+{
+
+int luax_ioError(lua_State *L, const char *fmt, ...)
 {
 	va_list args;
 	va_start(args, fmt);
@@ -36,13 +43,6 @@ static int ioError(lua_State *L, const char *fmt, ...)
 	return 2;
 }
 
-namespace love
-{
-namespace filesystem
-{
-namespace physfs
-{
-
 File *luax_checkfile(lua_State *L, int idx)
 {
 	return luax_checktype<File>(L, idx, "File", FILESYSTEM_FILE_T);
@@ -54,11 +54,12 @@ int w_File_getSize(lua_State *L)
 	int64 size = t->getSize();
 
 	// Push nil on failure or if size does not fit into a double precision floating-point number.
-	if (size == -1 || size >= 0x20000000000000LL)
-		lua_pushnil(L);
-	else
-		lua_pushnumber(L, (lua_Number)size);
+	if (size == -1)
+		return luax_ioError(L, "Could not determine file size.");
+	else if (size >= 0x20000000000000LL)
+		return luax_ioError(L, "Size is too large.");
 
+	lua_pushnumber(L, (lua_Number) size);
 	return 1;
 }
 
@@ -77,7 +78,7 @@ int w_File_open(lua_State *L)
 	}
 	catch (love::Exception &e)
 	{
-		return ioError(L, "%s", e.what());
+		return luax_ioError(L, "%s", e.what());
 	}
 
 	return 1;
@@ -110,7 +111,7 @@ int w_File_read(lua_State *L)
 	}
 	catch (love::Exception &e)
 	{
-		return ioError(L, "%s", e.what());
+		return luax_ioError(L, "%s", e.what());
 	}
 
 	lua_pushlstring(L, (const char *) d->getData(), d->getSize());
@@ -128,11 +129,17 @@ int w_File_write(lua_State *L)
 	{
 		try
 		{
-			result = file->write(lua_tostring(L, 2), luaL_optint(L, 3, lua_objlen(L, 2)));
+			size_t datasize = 0;
+			const char *data = lua_tolstring(L, 2, &datasize);
+
+			if (!lua_isnoneornil(L, 3))
+				datasize = luaL_checkinteger(L, 3);
+
+			result = file->write(data, datasize);
 		}
 		catch (love::Exception &e)
 		{
-			return ioError(L, "%s", e.what());
+			return luax_ioError(L, "%s", e.what());
 		}
 	}
 	else if (luax_istype(L, 2, DATA_T))
@@ -140,11 +147,11 @@ int w_File_write(lua_State *L)
 		try
 		{
 			love::Data *data = luax_totype<love::Data>(L, 2, "Data", DATA_T);
-			result = file->write(data, luaL_optint(L, 3, data->getSize()));
+			result = file->write(data, luaL_optinteger(L, 3, data->getSize()));
 		}
 		catch (love::Exception &e)
 		{
-			return ioError(L, "%s", e.what());
+			return luax_ioError(L, "%s", e.what());
 		}
 	}
 	else
@@ -152,7 +159,7 @@ int w_File_write(lua_State *L)
 		return luaL_argerror(L, 2, "string or data expected");
 	}
 
-	lua_pushboolean(L, result);
+	luax_pushboolean(L, result);
 	return 1;
 }
 
@@ -166,7 +173,7 @@ int w_File_flush(lua_State *L)
 	}
 	catch (love::Exception &e)
 	{
-		return ioError(L, "%s", e.what());
+		return luax_ioError(L, "%s", e.what());
 	}
 	luax_pushboolean(L, success);
 	return 1;
@@ -184,8 +191,10 @@ int w_File_tell(lua_State *L)
 	File *file = luax_checkfile(L, 1);
 	int64 pos = file->tell();
 	// Push nil on failure or if pos does not fit into a double precision floating-point number.
-	if (pos == -1 || pos >= 0x20000000000000LL)
-		lua_pushnil(L);
+	if (pos == -1)
+		return luax_ioError(L, "Invalid position.");
+	else if (pos >= 0x20000000000000LL)
+		return luax_ioError(L, "Number is too large.");
 	else
 		lua_pushnumber(L, (lua_Number)pos);
 	return 1;
@@ -217,15 +226,11 @@ int w_File_lines(lua_State *L)
 		if (file->getMode() != File::CLOSED)
 			file->close();
 
-		try
-		{
-			if (!file->open(File::READ))
-				return luaL_error(L, "Could not open file.");
-		}
-		catch (love::Exception &e)
-		{
-			return luaL_error(L, "%s", e.what());
-		}
+		bool success = false;
+		EXCEPT_GUARD(success = file->open(File::READ);)
+
+		if (!success)
+			return luaL_error(L, "Could not open file.");
 	}
 
 	lua_pushcclosure(L, Filesystem::lines_i, 3);
@@ -249,7 +254,7 @@ int w_File_setBuffer(lua_State *L)
 	}
 	catch (love::Exception &e)
 	{
-		return ioError(L, "%s", e.what());
+		return luax_ioError(L, "%s", e.what());
 	}
 
 	luax_pushboolean(L, success);
@@ -264,7 +269,7 @@ int w_File_getBuffer(lua_State *L)
 	const char *str = 0;
 
 	if (!File::getConstant(bufmode, str))
-		return ioError(L, "Unknown file buffer mode.");
+		return luax_ioError(L, "Unknown file buffer mode.");
 
 	lua_pushstring(L, str);
 	lua_pushnumber(L, (lua_Number) size);
@@ -279,7 +284,7 @@ int w_File_getMode(lua_State *L)
 	const char *str = 0;
 
 	if (!File::getConstant(mode, str))
-		return ioError(L, "Unknown file mode.");
+		return luax_ioError(L, "Unknown file mode.");
 
 	lua_pushstring(L, str);
 	return 1;

src/modules/filesystem/physfs/wrap_File.h

@@ -33,6 +33,9 @@ namespace filesystem
 namespace physfs
 {
 
+// Does not use lua_error, so it's safe to call in exception handling code.
+int luax_ioError(lua_State *L, const char *fmt, ...);
+
 File *luax_checkfile(lua_State *L, int idx);
 int w_File_getSize(lua_State *L);
 int w_File_open(lua_State *L);

src/modules/filesystem/physfs/wrap_Filesystem.cpp

@@ -21,17 +21,8 @@
 // LOVE
 #include "wrap_Filesystem.h"
 
-static int ioError(lua_State *L, const char *fmt, ...)
-{
-	va_list args;
-	va_start(args, fmt);
-
-	lua_pushnil(L);
-	lua_pushvfstring(L, fmt, args);
-
-	va_end(args);
-	return 2;
-}
+// SDL
+#include <SDL_loadso.h>
 
 namespace love
 {
@@ -52,16 +43,7 @@ bool hack_setupWriteDirectory()
 int w_init(lua_State *L)
 {
 	const char *arg0 = luaL_checkstring(L, 1);
-
-	try
-	{
-		instance->init(arg0);
-	}
-	catch(Exception &e)
-	{
-		return luaL_error(L, e.what());
-	}
-
+	EXCEPT_GUARD(instance->init(arg0);)
 	return 0;
 }
 
@@ -166,7 +148,7 @@ int w_newFile(lua_State *L)
 		catch (love::Exception &e)
 		{
 			t->release();
-			return ioError(L, "%s", e.what());
+			return luax_ioError(L, "%s", e.what());
 		}
 	}
 
@@ -194,7 +176,7 @@ int w_newFileData(lua_State *L)
 			}
 			catch (love::Exception &e)
 			{
-				return ioError(L, "%s", e.what());
+				return luax_ioError(L, "%s", e.what());
 			}
 			luax_pushtype(L, "FileData", FILESYSTEM_FILE_DATA_T, data);
 			return 1;
@@ -308,11 +290,11 @@ int w_read(lua_State *L)
 	}
 	catch (love::Exception &e)
 	{
-		return ioError(L, "%s", e.what());
+		return luax_ioError(L, "%s", e.what());
 	}
 
 	if (data == 0)
-		return ioError(L, "File could not be read.");
+		return luax_ioError(L, "File could not be read.");
 
 	// Push the string.
 	lua_pushlstring(L, (const char *) data->getData(), data->getSize());
@@ -356,11 +338,10 @@ static int w_write_or_append(lua_State *L, File::Mode mode)
 	}
 	catch (love::Exception &e)
 	{
-		return ioError(L, "%s", e.what());
+		return luax_ioError(L, "%s", e.what());
 	}
 
 	luax_pushboolean(L, true);
-
 	return 1;
 }
 
@@ -386,19 +367,17 @@ int w_lines(lua_State *L)
 	if (lua_isstring(L, 1))
 	{
 		file = instance->newFile(lua_tostring(L, 1));
-		try
-		{
-			if (!file->open(File::READ))
-				return luaL_error(L, "Could not open file.");
-		}
-		catch(love::Exception &e)
-		{
-			return luaL_error(L, "%s", e.what());
-		}
+		bool success = false;
+
+		EXCEPT_GUARD(success = file->open(File::READ);)
+
+		if (!success)
+			return luaL_error(L, "Could not open file.");
+		
 		luax_pushtype(L, "File", FILESYSTEM_FILE_T, file);
 	}
 	else
-		return luaL_error(L, "Expected filename.");
+		return luaL_argerror(L, 1, "expected filename.");
 
 	lua_pushcclosure(L, Filesystem::lines_i, 1);
 	return 1;
@@ -415,7 +394,7 @@ int w_load(lua_State *L)
 	}
 	catch (love::Exception &e)
 	{
-		return ioError(L, "%s", e.what());
+		return luax_ioError(L, "%s", e.what());
 	}
 
 	int status = luaL_loadbuffer(L, (const char *)data->getData(), data->getSize(), ("@" + filename).c_str());
@@ -445,13 +424,10 @@ int w_getLastModified(lua_State *L)
 	}
 	catch (love::Exception &e)
 	{
-		lua_pushnil(L);
-		lua_pushstring(L, e.what());
-		return 2;
+		return luax_ioError(L, "%s", e.what());
 	}
 
 	lua_pushnumber(L, static_cast<lua_Number>(time));
-
 	return 1;
 }
 
@@ -466,15 +442,14 @@ int w_getSize(lua_State *L)
 	}
 	catch (love::Exception &e)
 	{
-		return ioError(L, "%s", e.what());
+		return luax_ioError(L, "%s", e.what());
 	}
 
 	// Error on failure or if size does not fit into a double precision floating-point number.
 	if (size == -1)
-		return ioError(L, "Could not determine file size.");
+		return luax_ioError(L, "Could not determine file size.");
 	else if (size >= 0x20000000000000LL)
-		return luaL_error(L, "Size too large to fit into a Lua number!");
-
+		return luax_ioError(L, "Size too large to fit into a Lua number!");
 
 	lua_pushnumber(L, (lua_Number) size);
 	return 1;
@@ -629,23 +604,13 @@ extern "C" int luaopen_love_filesystem(lua_State *L)
 {
 	if (instance == 0)
 	{
-		try
-		{
-			instance = new Filesystem();
-			love::luax_register_searcher(L, loader, 1);
-			love::luax_register_searcher(L, extloader, 2);
-		}
-		catch(Exception &e)
-		{
-			return luaL_error(L, e.what());
-		}
+		EXCEPT_GUARD(instance = new Filesystem();)
 	}
 	else
-	{
 		instance->retain();
-		love::luax_register_searcher(L, loader, 1);
-		love::luax_register_searcher(L, extloader, 2);
-	}
+
+	love::luax_register_searcher(L, loader, 1);
+	love::luax_register_searcher(L, extloader, 2);
 
 	WrappedModule w;
 	w.module = instance;

src/modules/filesystem/physfs/wrap_Filesystem.h

@@ -26,9 +26,6 @@
 #include "wrap_File.h"
 #include "wrap_FileData.h"
 
-// SDL
-#include <SDL_loadso.h>
-
 namespace love
 {
 namespace filesystem