Merge pull request #7526 from ethereum/modifierDepth

Add modifier depth to source mappings.

Changelog.md

@@ -14,6 +14,10 @@ Breaking changes:
  * Syntax: Abstract contracts need to be marked explicitly as abstract by using the ``abstract`` keyword.
  * Inline Assembly: Only strict inline assembly is allowed.
  * Type checker: Resulting type of exponentiation is equal to the type of the base. Also allow signed types for the base.
+ * Source mappings: Add "modifier depth" as a fifth field in the source mappings.
+
+
+
  * Language Feature: When overriding a function or modifier, the new keyword ``override`` must be used. When overriding a function or modifier defined in multiple parallel bases, all bases must be listed in parentheses after the keyword like so: ``override(Base1, Base2)``
 
 Language Features:

docs/miscellaneous.rst

@@ -222,7 +222,8 @@ Furthermore, the compiler can also generate a mapping from the bytecode
 to the range in the source code that generated the instruction. This is again
 important for static analysis tools that operate on bytecode level and
 for displaying the current position in the source code inside a debugger
-or for breakpoint handling.
+or for breakpoint handling. This mapping also contains other information,
+like the jump type and the modifier depth (see below).
 
 Both kinds of source mappings use integer identifiers to refer to source files.
 The identifier of a source file is stored in
@@ -244,12 +245,17 @@ Where ``s`` is the byte-offset to the start of the range in the source file,
 index mentioned above.
 
 The encoding in the source mapping for the bytecode is more complicated:
-It is a list of ``s:l:f:j`` separated by ``;``. Each of these
+It is a list of ``s:l:f:j:m`` separated by ``;``. Each of these
 elements corresponds to an instruction, i.e. you cannot use the byte offset
 but have to use the instruction offset (push instructions are longer than a single byte).
-The fields ``s``, ``l`` and ``f`` are as above and ``j`` can be either
+The fields ``s``, ``l`` and ``f`` are as above. ``j`` can be either
 ``i``, ``o`` or ``-`` signifying whether a jump instruction goes into a
 function, returns from a function or is a regular jump as part of e.g. a loop.
+The last field, ``m``, is an integer that denotes the "modifier depth". This depth
+is increased whenever the placeholder statement (``_``) is entered in a modifier
+and decreased when it is left again. This allows debuggers to track tricky cases
+like the same modifier being used twice or multiple placeholder statements being
+used in a single modifier.
 
 In order to compress these source mappings especially for bytecode, the
 following rules are used:

libevmasm/Assembly.cpp

@@ -86,6 +86,7 @@ AssemblyItem const& Assembly::append(AssemblyItem const& _i)
 	m_items.emplace_back(_i);
 	if (m_items.back().location().isEmpty() && !m_currentSourceLocation.isEmpty())
 		m_items.back().setLocation(m_currentSourceLocation);
+	m_items.back().m_modifierDepth = m_currentModifierDepth;
 	return back();
 }
 

libevmasm/Assembly.h

@@ -181,6 +181,8 @@ protected:
 	int m_deposit = 0;
 
 	langutil::SourceLocation m_currentSourceLocation;
+public:
+	size_t m_currentModifierDepth = 0;
 };
 
 inline std::ostream& operator<<(std::ostream& _out, Assembly const& _a)

libevmasm/AssemblyItem.h

@@ -146,6 +146,8 @@ public:
 
 	std::string toAssemblyText() const;
 
+	size_t m_modifierDepth = 0;
+
 private:
 	AssemblyItemType m_type;
 	Instruction m_instruction; ///< Only valid if m_type == Operation

libsolidity/codegen/CompilerContext.h

@@ -262,6 +262,8 @@ public:
 			ScopeGuard([&]{ _compilerContext.popVisitedNodes(); }) { _compilerContext.pushVisitedNodes(&_node); }
 	};
 
+	void setModifierDepth(size_t _modifierDepth) { m_asm->m_currentModifierDepth = _modifierDepth; }
+
 private:
 	/// Searches the inheritance hierarchy towards the base starting from @a _searchStart and returns
 	/// the first function definition that is overwritten by _function.

libsolidity/codegen/ContractCompiler.cpp

@@ -564,8 +564,10 @@ bool ContractCompiler::visit(FunctionDefinition const& _function)
 	m_currentFunction = &_function;
 	m_modifierDepth = -1;
 	m_scopeStackHeight.clear();
+	m_context.setModifierDepth(0);
 
 	appendModifierOrFunctionCode();
+	m_context.setModifierDepth(0);
 	solAssert(m_returnTags.empty(), "");
 
 	// Now we need to re-shuffle the stack. For this we keep a record of the stack layout
@@ -1240,6 +1242,7 @@ void ContractCompiler::appendModifierOrFunctionCode()
 	vector<VariableDeclaration const*> addedVariables;
 
 	m_modifierDepth++;
+	m_context.setModifierDepth(m_modifierDepth);
 
 	if (m_modifierDepth >= m_currentFunction->modifiers().size())
 	{
@@ -1293,6 +1296,7 @@ void ContractCompiler::appendModifierOrFunctionCode()
 			m_context.removeVariable(*var);
 	}
 	m_modifierDepth--;
+	m_context.setModifierDepth(m_modifierDepth);
 }
 
 void ContractCompiler::appendStackVariableInitialisation(VariableDeclaration const& _variable)

libsolidity/interface/CompilerStack.cpp

@@ -1305,6 +1305,7 @@ string CompilerStack::computeSourceMapping(eth::AssemblyItems const& _items) con
 	int prevStart = -1;
 	int prevLength = -1;
 	int prevSourceIndex = -1;
+	size_t prevModifierDepth = -1;
 	char prevJump = 0;
 	for (auto const& item: _items)
 	{
@@ -1322,19 +1323,24 @@ string CompilerStack::computeSourceMapping(eth::AssemblyItems const& _items) con
 			jump = 'i';
 		else if (item.getJumpType() == eth::AssemblyItem::JumpType::OutOfFunction)
 			jump = 'o';
+		size_t modifierDepth = item.m_modifierDepth;
 
-		unsigned components = 4;
-		if (jump == prevJump)
+		unsigned components = 5;
+		if (modifierDepth == prevModifierDepth)
 		{
 			components--;
-			if (sourceIndex == prevSourceIndex)
+			if (jump == prevJump)
 			{
 				components--;
-				if (length == prevLength)
+				if (sourceIndex == prevSourceIndex)
 				{
 					components--;
-					if (location.start == prevStart)
+					if (length == prevLength)
+					{
 						components--;
+						if (location.start == prevStart)
+							components--;
+					}
 				}
 			}
 		}
@@ -1358,6 +1364,12 @@ string CompilerStack::computeSourceMapping(eth::AssemblyItems const& _items) con
 						ret += ':';
 						if (jump != prevJump)
 							ret += jump;
+						if (components-- > 0)
+						{
+							ret += ':';
+							if (modifierDepth != prevModifierDepth)
+								ret += to_string(modifierDepth);
+						}
 					}
 				}
 			}
@@ -1367,6 +1379,7 @@ string CompilerStack::computeSourceMapping(eth::AssemblyItems const& _items) con
 		prevLength = length;
 		prevSourceIndex = sourceIndex;
 		prevJump = jump;
+		prevModifierDepth = modifierDepth;
 	}
 	return ret;
 }