diff --git a/Changelog.md b/Changelog.md index f01abd6ab..9d8a942ae 100644 --- a/Changelog.md +++ b/Changelog.md @@ -23,6 +23,7 @@ Bugfixes: * Solidity Upgrade Tool ``solidity-upgrade``: Fix the tool returning success code on uncaught exceptions. * SMTChecker: Fix display error for negative integers that are one more than powers of two. * SMTChecker: Improved readability for large integers that are powers of two or almost powers of two in error messages. + * SMTChecker: Fix internal error when a public library function is called internally. ### 0.8.17 (2022-09-08) diff --git a/libsolidity/formal/CHC.cpp b/libsolidity/formal/CHC.cpp index 35c403327..2baedde12 100644 --- a/libsolidity/formal/CHC.cpp +++ b/libsolidity/formal/CHC.cpp @@ -1172,7 +1172,14 @@ void CHC::defineInterfacesAndSummaries(SourceUnit const& _source) m_summaries[contract].emplace(function, createSummaryBlock(*function, *contract)); - if (!function->isConstructor() && function->isPublic() && resolved.count(function)) + if ( + !function->isConstructor() && + function->isPublic() && + // Public library functions should have interfaces only for the libraries + // they're declared in. + (!function->libraryFunction() || (function->scope() == contract)) && + resolved.count(function) + ) { m_externalSummaries[contract].emplace(function, createSummaryBlock(*function, *contract)); diff --git a/libsolidity/formal/SMTEncoder.cpp b/libsolidity/formal/SMTEncoder.cpp index 4b267bc81..818ac9278 100644 --- a/libsolidity/formal/SMTEncoder.cpp +++ b/libsolidity/formal/SMTEncoder.cpp @@ -3162,11 +3162,10 @@ void SMTEncoder::collectFreeFunctions(set(node.get()); contract && contract->isLibrary() ) - { for (auto function: contract->definedFunctions()) - if (!function->isPublic()) - m_freeFunctions.insert(function); - } + // We need to add public library functions too because they can be called + // internally by internal library functions that are considered free functions. + m_freeFunctions.insert(function); } void SMTEncoder::createFreeConstants(set const& _sources) diff --git a/test/libsolidity/SMTCheckerTest.cpp b/test/libsolidity/SMTCheckerTest.cpp index 5bc9a0be9..1d6109176 100644 --- a/test/libsolidity/SMTCheckerTest.cpp +++ b/test/libsolidity/SMTCheckerTest.cpp @@ -29,6 +29,10 @@ using namespace solidity::frontend::test; SMTCheckerTest::SMTCheckerTest(string const& _filename): SyntaxTest(_filename, EVMVersion{}) { + auto contract = m_reader.stringSetting("SMTContract", ""); + if (!contract.empty()) + m_modelCheckerSettings.contracts.contracts[""] = {contract}; + auto const& showUnproved = m_reader.stringSetting("SMTShowUnproved", "yes"); if (showUnproved == "no") m_modelCheckerSettings.showUnproved = false; diff --git a/test/libsolidity/smtCheckerTests/functions/library_public_called_as_internal_1.sol b/test/libsolidity/smtCheckerTests/functions/library_public_called_as_internal_1.sol new file mode 100644 index 000000000..fd763b8c5 --- /dev/null +++ b/test/libsolidity/smtCheckerTests/functions/library_public_called_as_internal_1.sol @@ -0,0 +1,26 @@ +library L { + function f1(uint x) public pure { + assert(x > 0); // should fail + } + function f(uint x) internal pure { f1(x); } +} + +contract C { + function g(uint x) external pure { + // This should trigger the assertion failure + // since it calls `f` internally, which calls + // `f1` internally. + return L.f(x); + } + + function h(uint x) external pure { + // This should not trigger the assertion failure + // since it delegatecalls and that's not supported. + return L.f1(x); + } +} +// ==== +// SMTContract: C +// ---- +// Warning 4588: (533-540): Assertion checker does not yet implement this type of function call. +// Warning 6328: (58-71): CHC: Assertion violation happens here.\nCounterexample:\n\nx = 0\n\nTransaction trace:\nC.constructor()\nC.g(0)\n L.f(0) -- internal call\n L.f1(0) -- internal call diff --git a/test/libsolidity/smtCheckerTests/functions/library_public_called_as_internal_2.sol b/test/libsolidity/smtCheckerTests/functions/library_public_called_as_internal_2.sol new file mode 100644 index 000000000..c0c62c246 --- /dev/null +++ b/test/libsolidity/smtCheckerTests/functions/library_public_called_as_internal_2.sol @@ -0,0 +1,24 @@ +library L { + function f1(uint x) public pure { + assert(x > 0); // should fail + } + function f() internal pure { + f1(0); // should cause the assertion in `f1` to fail + } + function g() internal pure { + f1(1); // should not cause the assertion in `f1` to fail + } +} + +contract C { + function f() external pure { + return L.f(); // should cause the assertion to fail + } + function g() external pure { + return L.g(); // should not cause the assertion to fail + } +} +// ==== +// SMTContract: C +// ---- +// Warning 6328: (58-71): CHC: Assertion violation happens here.\nCounterexample:\n\n\nTransaction trace:\nC.constructor()\nC.f()\n L.f() -- internal call\n L.f1(0) -- internal call