From bc875f6b9c140d351ec8ae96d2c8e29979017d57 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 5 Dec 2017 10:44:28 +0000 Subject: [PATCH 1/3] Warn for assembly labels too --- Changelog.md | 1 + libsolidity/inlineasm/AsmAnalysis.cpp | 5 +++-- test/libsolidity/InlineAssembly.cpp | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Changelog.md b/Changelog.md index fd256af8a..248165bbc 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,6 +1,7 @@ ### 0.4.20 (unreleased) Features: + * Inline Assembly: Issue warning for using jump labels (already existed for jump instructions). Bugfixes: diff --git a/libsolidity/inlineasm/AsmAnalysis.cpp b/libsolidity/inlineasm/AsmAnalysis.cpp index 5c03342d0..04b8417f0 100644 --- a/libsolidity/inlineasm/AsmAnalysis.cpp +++ b/libsolidity/inlineasm/AsmAnalysis.cpp @@ -56,6 +56,7 @@ bool AsmAnalyzer::operator()(Label const& _label) { solAssert(!m_julia, ""); m_info.stackHeightInfo[&_label] = m_stackHeight; + warnOnInstructions(solidity::Instruction::JUMPDEST, _label.location); return true; } @@ -523,10 +524,10 @@ void AsmAnalyzer::warnOnInstructions(solidity::Instruction _instr, SourceLocatio "the Metropolis hard fork. Before that it acts as an invalid instruction." ); - if (_instr == solidity::Instruction::JUMP || _instr == solidity::Instruction::JUMPI) + if (_instr == solidity::Instruction::JUMP || _instr == solidity::Instruction::JUMPI || _instr == solidity::Instruction::JUMPDEST) m_errorReporter.warning( _location, - "Jump instructions are low-level EVM features that can lead to " + "Jump instructions and labels are low-level EVM features that can lead to " "incorrect stack access. Because of that they are discouraged. " "Please consider using \"switch\" or \"for\" statements instead." ); diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp index e9fb84316..506721c1d 100644 --- a/test/libsolidity/InlineAssembly.cpp +++ b/test/libsolidity/InlineAssembly.cpp @@ -714,6 +714,7 @@ BOOST_AUTO_TEST_CASE(jump_warning) CHECK_PARSE_WARNING("{ 1 2 jumpi }", Warning, "Jump instructions"); CHECK_PARSE_WARNING("{ a: jump(a) }", Warning, "Jump instructions"); CHECK_PARSE_WARNING("{ a: jumpi(a, 2) }", Warning, "Jump instructions"); + CHECK_PARSE_WARNING("{ a: }", Warning, "Jump instructions"); } BOOST_AUTO_TEST_SUITE_END() From 793537e08926190728e93a666a5fadc6d0765f63 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 5 Dec 2017 10:45:44 +0000 Subject: [PATCH 2/3] Suggest the "if" statement too instead of jumps --- libsolidity/inlineasm/AsmAnalysis.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libsolidity/inlineasm/AsmAnalysis.cpp b/libsolidity/inlineasm/AsmAnalysis.cpp index 04b8417f0..049af65fb 100644 --- a/libsolidity/inlineasm/AsmAnalysis.cpp +++ b/libsolidity/inlineasm/AsmAnalysis.cpp @@ -529,6 +529,6 @@ void AsmAnalyzer::warnOnInstructions(solidity::Instruction _instr, SourceLocatio _location, "Jump instructions and labels are low-level EVM features that can lead to " "incorrect stack access. Because of that they are discouraged. " - "Please consider using \"switch\" or \"for\" statements instead." + "Please consider using \"switch\", \"if\" or \"for\" statements instead." ); } From d57afb20fa32be3d5575b8c5dcd98f39ce4e90d0 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 5 Dec 2017 20:27:50 +0000 Subject: [PATCH 3/3] Fix warning test for jumps in assembly --- test/libsolidity/InlineAssembly.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp index 506721c1d..82bafd49d 100644 --- a/test/libsolidity/InlineAssembly.cpp +++ b/test/libsolidity/InlineAssembly.cpp @@ -712,8 +712,8 @@ BOOST_AUTO_TEST_CASE(jump_warning) { CHECK_PARSE_WARNING("{ 1 jump }", Warning, "Jump instructions"); CHECK_PARSE_WARNING("{ 1 2 jumpi }", Warning, "Jump instructions"); - CHECK_PARSE_WARNING("{ a: jump(a) }", Warning, "Jump instructions"); - CHECK_PARSE_WARNING("{ a: jumpi(a, 2) }", Warning, "Jump instructions"); + CHECK_PARSE_WARNING("{ jump(44) }", Warning, "Jump instructions"); + CHECK_PARSE_WARNING("{ jumpi(44, 2) }", Warning, "Jump instructions"); CHECK_PARSE_WARNING("{ a: }", Warning, "Jump instructions"); }