Fix segfault for empty @return tags in modifiers

Changelog.md

@@ -18,6 +18,7 @@ Compiler Features:
 
 Bugfixes:
  * Code generator: Do not pad empty string literals with a single 32-byte zero field in the ABI coder v1.
+ * NatSpec: Fix segfault when inheriting return parameter documentation for modifiers with no parameters.
  * SMTChecker: Fix cast string literals to byte arrays.
  * SMTChecker: Fix internal compiler error when doing bitwise compound assignment with string literals.
  * SMTChecker: Fix internal error when trying to generate counterexamples with old z3.

libsolidity/analysis/DocStringAnalyser.cpp

@@ -42,7 +42,15 @@ void copyMissingTags(set<CallableDeclaration const*> const& _baseFunctions, Stru
 	if (_baseFunctions.size() != 1)
 		return;
 
-	auto& sourceDoc = dynamic_cast<StructurallyDocumentedAnnotation const&>((*_baseFunctions.begin())->annotation());
+	CallableDeclaration const& baseFunction = **_baseFunctions.begin();
+
+	auto hasReturnParameter = [](CallableDeclaration const& declaration, size_t _n)
+	{
+		return declaration.returnParameterList() &&
+			declaration.returnParameters().size() > _n;
+	};
+
+	auto& sourceDoc = dynamic_cast<StructurallyDocumentedAnnotation const&>(baseFunction.annotation());
 
 	for (auto it = sourceDoc.docTags.begin(); it != sourceDoc.docTags.end();)
 	{
@@ -66,9 +74,15 @@ void copyMissingTags(set<CallableDeclaration const*> const& _baseFunctions, Stru
 				size_t docParaNameEndPos = content.content.find_first_of(" \t");
 				string const docParameterName = content.content.substr(0, docParaNameEndPos);
 
-				if (docParameterName != _declaration->returnParameters().at(n)->name())
+				if (
+					hasReturnParameter(*_declaration, n) &&
+					docParameterName != _declaration->returnParameters().at(n)->name()
+				)
 				{
-					bool baseHasNoName = (*_baseFunctions.begin())->returnParameters().at(n)->name().empty();
+					bool baseHasNoName =
+						hasReturnParameter(baseFunction, n) &&
+						baseFunction.returnParameters().at(n)->name().empty();
+
 					string paramName = _declaration->returnParameters().at(n)->name();
 					content.content =
 						(paramName.empty() ? "" : std::move(paramName) + " ") + (

libsolidity/analysis/DocStringTagParser.cpp

@@ -127,10 +127,13 @@ void DocStringTagParser::handleCallable(
 )
 {
 	static set<string> const validEventTags = set<string>{"dev", "notice", "return", "param"};
+	static set<string> const validModifierTags = set<string>{"dev", "notice", "param", "inheritdoc"};
 	static set<string> const validTags = set<string>{"dev", "notice", "return", "param", "inheritdoc"};
 
 	if (dynamic_cast<EventDefinition const*>(&_callable))
 		parseDocStrings(_node, _annotation, validEventTags, "events");
+	else if (dynamic_cast<ModifierDefinition const*>(&_callable))
+		parseDocStrings(_node, _annotation, validModifierTags, "modifiers");
 	else
 		parseDocStrings(_node, _annotation, validTags, "functions");
 

test/libsolidity/syntaxTests/natspec/invalid/docstring_inherit_modifier_no_return.sol

@@ -0,0 +1,10 @@
+contract C {
+	///@return
+	modifier m22 virtual { _; }
+}
+
+contract D is C {
+	modifier m22 override { _; }
+}
+// ----
+// DocstringParsingError 6546: (14-24): Documentation tag @return not valid for modifiers.

test/libsolidity/syntaxTests/natspec/invalid/docstring_inherit_modifier_no_return2.sol

@@ -0,0 +1,12 @@
+contract A {
+	/// @return a
+	function g(int x) public virtual { return 2; }
+}
+
+contract B is A {
+	function g(int x) public pure override returns (int b) { return 2; }
+}
+// ----
+// DocstringParsingError 2604: (14-27): Documentation tag "@return a" exceeds the number of return parameters.
+// TypeError 4822: (98-166): Overriding function return types differ.
+// TypeError 8863: (64-72): Different number of arguments in return statement than in returns declaration.