Merge pull request #7898 from ethereum/solc-mallocs

libsolc: memory allocation API cleanup

Changelog.md

@@ -6,6 +6,8 @@ Breaking changes:
  * AST: Inline assembly is exported as structured JSON instead of plain string.
  * C API (``libsolc``): Introduce context parameter to both ``solidity_compile`` and the callback.
  * C API (``libsolc``): The provided callback now takes two parameters, kind and data. The callback can then be used for multiple purposes, such has file imports and SMT queries.
+ * C API (``libsolc``): ``solidity_free`` was renamed to ``solidity_reset``. Functions ``solidity_alloc`` and ``solidity_free`` were added.
+ * C API (``libsolc``): ``solidity_compile`` now returns a string that must be explicitly freed via ``solidity_free()``
  * Commandline Interface: Remove the text-based AST printer (``--ast``).
  * Commandline Interface: Switch to the new error reporter by default. ``--old-reporter`` falls back to the deprecated old error reporter.
  * Commandline Interface: Add option to disable or choose hash method between IPFS and Swarm for the bytecode metadata.

libsolc/CMakeLists.txt

@@ -2,7 +2,7 @@ if (EMSCRIPTEN)
 	# Specify which functions to export in soljson.js.
 	# Note that additional Emscripten-generated methods needed by solc-js are
 	# defined to be exported in cmake/EthCompilerSettings.cmake.
-	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -s EXPORTED_FUNCTIONS='[\"_solidity_license\",\"_solidity_version\",\"_solidity_compile\"]' -s RESERVED_FUNCTION_POINTERS=20")
+	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -s EXPORTED_FUNCTIONS='[\"_solidity_license\",\"_solidity_version\",\"_solidity_compile\",\"_solidity_alloc\",\"_solidity_free\",\"_solidity_reset\"]' -s RESERVED_FUNCTION_POINTERS=20")
 	add_executable(soljson libsolc.cpp libsolc.h)
 	target_link_libraries(soljson PRIVATE solidity)
 else()

libsolc/libsolc.cpp

@@ -27,6 +27,8 @@
 #include <libdevcore/Common.h>
 #include <libdevcore/JSON.h>
 
+#include <cstdlib>
+#include <list>
 #include <string>
 
 #include "license.h"
@@ -38,6 +40,36 @@ using namespace solidity;
 namespace
 {
 
+// The strings in this list must not be resized after they have been added here (via solidity_alloc()), because
+// this may potentially change the pointer that was passed to the caller from solidity_alloc().
+static list<string> solidityAllocations;
+
+/// Find the equivalent to @p _data in the list of allocations of solidity_alloc(),
+/// removes it from the list and returns its value.
+///
+/// If any invalid argument is being passed, it is considered a programming error
+/// on the caller-side and hence, will call abort() then.
+string takeOverAllocation(char const* _data)
+{
+	for (auto iter = begin(solidityAllocations); iter != end(solidityAllocations); ++iter)
+		if (iter->data() == _data)
+		{
+			string chunk = move(*iter);
+			solidityAllocations.erase(iter);
+			return chunk;
+		}
+
+	abort();
+}
+
+/// Resizes a std::string to the proper length based on the occurrence of a zero terminator.
+void truncateCString(string& _data)
+{
+	size_t pos = _data.find('\0');
+	if (pos != string::npos)
+		_data.resize(pos);
+}
+
 ReadCallback::Callback wrapReadCallback(CStyleReadFileCallback _readCallback, void* _readContext)
 {
 	ReadCallback::Callback readCallback;
@@ -47,7 +79,7 @@ ReadCallback::Callback wrapReadCallback(CStyleReadFileCallback _readCallback, vo
 		{
 			char* contents_c = nullptr;
 			char* error_c = nullptr;
-			_readCallback(_readContext, _kind.c_str(), _data.c_str(), &contents_c, &error_c);
+			_readCallback(_readContext, _kind.data(), _data.data(), &contents_c, &error_c);
 			ReadCallback::Result result;
 			result.success = true;
 			if (!contents_c && !error_c)
@@ -58,15 +90,14 @@ ReadCallback::Callback wrapReadCallback(CStyleReadFileCallback _readCallback, vo
 			if (contents_c)
 			{
 				result.success = true;
-				result.responseOrErrorMessage = string(contents_c);
+				result.responseOrErrorMessage = takeOverAllocation(contents_c);
-				free(contents_c);
 			}
 			if (error_c)
 			{
 				result.success = false;
-				result.responseOrErrorMessage = string(error_c);
+				result.responseOrErrorMessage = takeOverAllocation(error_c);
-				free(error_c);
 			}
+			truncateCString(result.responseOrErrorMessage);
 			return result;
 		};
 	}
@@ -76,13 +107,11 @@ ReadCallback::Callback wrapReadCallback(CStyleReadFileCallback _readCallback, vo
 string compile(string _input, CStyleReadFileCallback _readCallback, void* _readContext)
 {
 	StandardCompiler compiler(wrapReadCallback(_readCallback, _readContext));
-	return compiler.compile(std::move(_input));
+	return compiler.compile(move(_input));
 }
 
 }
 
-static string s_outputBuffer;
-
 extern "C"
 {
 extern char const* solidity_license() noexcept
@@ -90,20 +119,40 @@ extern char const* solidity_license() noexcept
 	static string fullLicenseText = otherLicenses + licenseText;
 	return fullLicenseText.c_str();
 }
+
 extern char const* solidity_version() noexcept
 {
 	return VersionString.c_str();
 }
+
 extern char const* solidity_compile(char const* _input, CStyleReadFileCallback _readCallback, void* _readContext) noexcept
 {
-	s_outputBuffer = compile(_input, _readCallback, _readContext);
+	return solidityAllocations.emplace_back(compile(_input, _readCallback, _readContext)).data();
-	return s_outputBuffer.c_str();
 }
-extern void solidity_free() noexcept
+
+extern char* solidity_alloc(size_t _size) noexcept
+{
+	try
+	{
+		return solidityAllocations.emplace_back(_size, '\0').data();
+	}
+	catch (...)
+	{
+		// most likely a std::bad_alloc(), if at all.
+		return nullptr;
+	}
+}
+
+extern void solidity_free(char* _data) noexcept
+{
+	takeOverAllocation(_data);
+}
+
+extern void solidity_reset() noexcept
 {
 	// This is called right before each compilation, but not at the end, so additional memory
 	// can be freed here.
 	yul::YulStringRepository::reset();
-	s_outputBuffer.clear();
+	solidityAllocations.clear();
 }
 }

libsolc/libsolc.h

@@ -23,6 +23,7 @@
 #pragma once
 
 #include <stdbool.h>
+#include <stddef.h>
 
 #ifdef __cplusplus
 #define SOLC_NOEXCEPT noexcept
@@ -38,39 +39,61 @@ extern "C" {
 ///
 /// @param _context The readContext passed to solidity_compile. Can be NULL.
 /// @param _kind The kind of callback (a string).
-/// @param _data The data for the callback.
+/// @param _data The data for the callback (a string).
-/// @param o_contents A pointer to the contents of the file, if found.
+/// @param o_contents A pointer to the contents of the file, if found. Allocated via solidity_alloc().
 /// @param o_error A pointer to an error message, if there is one.
 ///
-/// If the callback is not supported, o_contents and o_error should be set to NULL.
+/// The file (as well as error) contents that is to be allocated by the callback
+/// implementor must use the solidity_alloc() API to allocate its underlying
+/// storage. Ownership is then transferred to the compiler which will take care
+/// of the deallocation.
 ///
-/// The two pointers (o_contents and o_error) should be heap-allocated and are free'd by the caller.
+/// If the callback is not supported, *o_contents and *o_error must be set to NULL.
 typedef void (*CStyleReadFileCallback)(void* _context, char const* _kind, char const* _data, char** o_contents, char** o_error);
 
 /// Returns the complete license document.
 ///
-/// The pointer returned must not be freed by the caller.
+/// The pointer returned must NOT be freed by the caller.
 char const* solidity_license() SOLC_NOEXCEPT;
 
 /// Returns the compiler version.
 ///
-/// The pointer returned must not be freed by the caller.
+/// The pointer returned must NOT be freed by the caller.
 char const* solidity_version() SOLC_NOEXCEPT;
 
+/// Allocates a chunk of memory of @p _size bytes.
+///
+/// Use this function inside callbacks to allocate data that is to be passed to
+/// the compiler. You may use solidity_free() or solidity_reset() to free this
+/// memory again but it is not required as the compiler takes ownership for any
+/// data passed to it via callbacks.
+///
+/// This function will return NULL if the requested memory region could not be allocated.
+char* solidity_alloc(size_t _size) SOLC_NOEXCEPT;
+
+/// Explicitly frees the memory (@p _data) that was being allocated with solidity_alloc()
+/// or returned by a call to solidity_compile().
+///
+/// Important, this call will abort() in case of any invalid argument being passed to this call.
+void solidity_free(char* _data) SOLC_NOEXCEPT;
+
 /// Takes a "Standard Input JSON" and an optional callback (can be set to null). Returns
 /// a "Standard Output JSON". Both are to be UTF-8 encoded.
 ///
 /// @param _input The input JSON to process.
-/// @param _readCallback The optional callback pointer. Can be NULL.
+/// @param _readCallback The optional callback pointer. Can be NULL, but if not NULL,
+///                      it can be called by the compiler to request additional input.
+///                      Please see the documentation of the type for details.
 /// @param _readContext An optional context pointer passed to _readCallback. Can be NULL.
 ///
-/// @returns A pointer to the result. The pointer returned must not be freed by the caller.
+/// @returns A pointer to the result. The pointer returned must be freed by the caller using solidity_free() or solidity_reset().
 char const* solidity_compile(char const* _input, CStyleReadFileCallback _readCallback, void* _readContext) SOLC_NOEXCEPT;
 
 /// Frees up any allocated memory.
 ///
-/// NOTE: the pointer returned by solidity_compile is invalid after calling this!
+/// NOTE: the pointer returned by solidity_compile as well as any other pointer retrieved via solidity_alloc()
-void solidity_free() SOLC_NOEXCEPT;
+/// is invalid after calling this!
+void solidity_reset() SOLC_NOEXCEPT;
 
 #ifdef __cplusplus
 }

test/libsolidity/LibSolc.cpp

@@ -60,12 +60,21 @@ bool containsError(Json::Value const& _compilerResult, string const& _type, stri
 Json::Value compile(string const& _input, CStyleReadFileCallback _callback = nullptr)
 {
 	string output(solidity_compile(_input.c_str(), _callback, nullptr));
+	// This should be safe given the above copies the output.
+	solidity_reset();
 	Json::Value ret;
 	BOOST_REQUIRE(jsonParseStrict(output, ret));
-	solidity_free();
 	return ret;
 }
 
+char* stringToSolidity(string const& _input)
+{
+	char* ptr = solidity_alloc(_input.length());
+	BOOST_REQUIRE(ptr != nullptr);
+	std::memcpy(ptr, _input.c_str(), _input.length());
+	return ptr;
+}
+
 } // end anonymous namespace
 
 BOOST_AUTO_TEST_SUITE(LibSolc)
@@ -74,14 +83,12 @@ BOOST_AUTO_TEST_CASE(read_version)
 {
 	string output(solidity_version());
 	BOOST_CHECK(output.find(VersionString) == 0);
-	solidity_free();
 }
 
 BOOST_AUTO_TEST_CASE(read_license)
 {
 	string output(solidity_license());
 	BOOST_CHECK(output.find("GNU GENERAL PUBLIC LICENSE") != string::npos);
-	solidity_free();
 }
 
 BOOST_AUTO_TEST_CASE(standard_compilation)
@@ -97,7 +104,7 @@ BOOST_AUTO_TEST_CASE(standard_compilation)
 	}
 	)";
 	Json::Value result = compile(input);
-	BOOST_CHECK(result.isObject());
+	BOOST_REQUIRE(result.isObject());
 
 	// Only tests some assumptions. The StandardCompiler is tested properly in another suite.
 	BOOST_CHECK(result.isMember("sources"));
@@ -119,7 +126,7 @@ BOOST_AUTO_TEST_CASE(missing_callback)
 	}
 	)";
 	Json::Value result = compile(input);
-	BOOST_CHECK(result.isObject());
+	BOOST_REQUIRE(result.isObject());
 
 	BOOST_CHECK(containsError(result, "ParserError", "Source \"missing.sol\" not found: File not supplied initially."));
 }
@@ -141,19 +148,19 @@ BOOST_AUTO_TEST_CASE(with_callback)
 		[](void* _context, char const* _kind, char const* _path, char** o_contents, char** o_error)
 		{
 			// Passed in a nullptr in the compile() helper above.
-			BOOST_CHECK(_context == nullptr);
+			BOOST_REQUIRE(_context == nullptr);
 			// Caller frees the pointers.
-			BOOST_CHECK(string(_kind) == ReadCallback::kindString(ReadCallback::Kind::ReadFile));
+			BOOST_REQUIRE(string(_kind) == ReadCallback::kindString(ReadCallback::Kind::ReadFile));
 			if (string(_path) == "found.sol")
 			{
 				static string content{"import \"missing.sol\"; contract B {}"};
-				*o_contents = strdup(content.c_str());
+				*o_contents = stringToSolidity(content);
 				*o_error = nullptr;
 			}
 			else if (string(_path) == "missing.sol")
 			{
 				static string errorMsg{"Missing file."};
-				*o_error = strdup(errorMsg.c_str());
+				*o_error = stringToSolidity(errorMsg);
 				*o_contents = nullptr;
 			}
 			else
@@ -165,7 +172,7 @@ BOOST_AUTO_TEST_CASE(with_callback)
 	};
 
 	Json::Value result = compile(input, callback);
-	BOOST_CHECK(result.isObject());
+	BOOST_REQUIRE(result.isObject());
 
 	// This ensures that "found.sol" was properly loaded which triggered the second import statement.
 	BOOST_CHECK(containsError(result, "ParserError", "Source \"missing.sol\" not found: Missing file."));

test/tools/fuzzer_common.cpp

@@ -47,7 +47,7 @@ void FuzzerUtil::runCompiler(string const& _input, bool _quiet)
 		cout << "Output JSON: " << outputString << endl;
 
 	// This should be safe given the above copies the output.
-	solidity_free();
+	solidity_reset();
 
 	Json::Value output;
 	if (!jsonParseStrict(outputString, output))