Skip to content

Store both Yul and Solidity locations in debug data #11998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 22, 2021
Merged

Store both Yul and Solidity locations in debug data #11998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
84ca67c
Yul syntax tests showing broken behavior
cameel Sep 21, 2021
9c1d40d
AsmParser: Replace m_debugDataOverride with separate variables for lo…
cameel Sep 21, 2021
ce4420f
Separate DebugData fields for Solidity and Yul source locations
cameel Sep 20, 2021
d23754e
AsmParser: Don't use locationOverride as current location when locati…
cameel Sep 21, 2021
fc7e8c5
Fill out originLocation with nativeLocation when importing Yul AST
cameel Sep 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions libsolidity/analysis/ControlFlowBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,8 @@ bool ControlFlowBuilder::visit(InlineAssembly const& _inlineAssembly)
void ControlFlowBuilder::visit(yul::Statement const& _statement)
{
solAssert(m_currentNode && m_inlineAssembly, "");
m_currentNode->location = langutil::SourceLocation::smallestCovering(m_currentNode->location, locationOf(_statement));
solAssert(nativeLocationOf(_statement) == originLocationOf(_statement), "");
m_currentNode->location = langutil::SourceLocation::smallestCovering(m_currentNode->location, nativeLocationOf(_statement));
ASTWalker::visit(_statement);
}

Expand Down Expand Up @@ -496,14 +497,15 @@ void ControlFlowBuilder::operator()(yul::Identifier const& _identifier)
solAssert(m_currentNode && m_inlineAssembly, "");
auto const& externalReferences = m_inlineAssembly->annotation().externalReferences;
if (externalReferences.count(&_identifier))
{
if (auto const* declaration = dynamic_cast<VariableDeclaration const*>(externalReferences.at(&_identifier).declaration))
{
solAssert(nativeLocationOf(_identifier) == originLocationOf(_identifier), "");
m_currentNode->variableOccurrences.emplace_back(
*declaration,
VariableOccurrence::Kind::Access,
_identifier.debugData->location
nativeLocationOf(_identifier)
);
}
}
}

void ControlFlowBuilder::operator()(yul::Assignment const& _assignment)
Expand All @@ -514,11 +516,14 @@ void ControlFlowBuilder::operator()(yul::Assignment const& _assignment)
for (auto const& variable: _assignment.variableNames)
if (externalReferences.count(&variable))
if (auto const* declaration = dynamic_cast<VariableDeclaration const*>(externalReferences.at(&variable).declaration))
{
solAssert(nativeLocationOf(variable) == originLocationOf(variable), "");
m_currentNode->variableOccurrences.emplace_back(
*declaration,
VariableOccurrence::Kind::Assignment,
variable.debugData->location
nativeLocationOf(variable)
);
}
}

void ControlFlowBuilder::operator()(yul::FunctionCall const& _functionCall)
Expand Down
22 changes: 14 additions & 8 deletions libsolidity/analysis/ReferencesResolver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,13 @@ bool ReferencesResolver::visit(Return const& _return)

void ReferencesResolver::operator()(yul::FunctionDefinition const& _function)
{
validateYulIdentifierName(_function.name, _function.debugData->location);
solAssert(nativeLocationOf(_function) == originLocationOf(_function), "");
validateYulIdentifierName(_function.name, nativeLocationOf(_function));
for (yul::TypedName const& varName: _function.parameters + _function.returnVariables)
validateYulIdentifierName(varName.name, varName.debugData->location);
{
solAssert(nativeLocationOf(varName) == originLocationOf(varName), "");
validateYulIdentifierName(varName.name, nativeLocationOf(varName));
}

bool wasInsideFunction = m_yulInsideFunction;
m_yulInsideFunction = true;
Expand All @@ -213,6 +217,8 @@ void ReferencesResolver::operator()(yul::FunctionDefinition const& _function)

void ReferencesResolver::operator()(yul::Identifier const& _identifier)
{
solAssert(nativeLocationOf(_identifier) == originLocationOf(_identifier), "");

static set<string> suffixes{"slot", "offset", "length"};
string suffix;
for (string const& s: suffixes)
Expand All @@ -238,7 +244,7 @@ void ReferencesResolver::operator()(yul::Identifier const& _identifier)
{
m_errorReporter.declarationError(
4718_error,
_identifier.debugData->location,
nativeLocationOf(_identifier),
"Multiple matching identifiers. Resolving overloaded identifiers is not supported."
);
Comment on lines 245 to 249
Copy link
Collaborator Author

@cameel cameel Sep 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion I added for this is actually failing during AST export and reimport of syntaxTests/inlineAssembly/storage_reference_old_shadow.sol:

contract C {
    uint[] x;
    fallback() external {
        uint y_slot = 2;
        uint y_offset = 3;
        uint[] storage y = x;
        assembly {
            pop(y_slot)
            pop(y_offset)
        }
        y[0] = 2;
    }
}
// ----

This is why CLI tests are failing. BTW the error handling for AST import is swallowing errors at multiple levels (both in solc and in the test script), which makes it harder to debug than it should be.

return;
Expand All @@ -251,7 +257,7 @@ void ReferencesResolver::operator()(yul::Identifier const& _identifier)
)
m_errorReporter.declarationError(
9467_error,
_identifier.debugData->location,
nativeLocationOf(_identifier),
"Identifier not found. Use \".slot\" and \".offset\" to access storage variables."
);
return;
Expand All @@ -261,7 +267,7 @@ void ReferencesResolver::operator()(yul::Identifier const& _identifier)
{
m_errorReporter.declarationError(
6578_error,
_identifier.debugData->location,
nativeLocationOf(_identifier),
"Cannot access local Solidity variables from inside an inline assembly function."
);
return;
Expand All @@ -275,8 +281,8 @@ void ReferencesResolver::operator()(yul::VariableDeclaration const& _varDecl)
{
for (auto const& identifier: _varDecl.variables)
{
validateYulIdentifierName(identifier.name, identifier.debugData->location);

solAssert(nativeLocationOf(identifier) == originLocationOf(identifier), "");
validateYulIdentifierName(identifier.name, nativeLocationOf(identifier));

if (
auto declarations = m_resolver.nameFromCurrentScope(identifier.name.str());
Expand All @@ -289,7 +295,7 @@ void ReferencesResolver::operator()(yul::VariableDeclaration const& _varDecl)
if (!ssl.infos.empty())
m_errorReporter.declarationError(
3859_error,
identifier.debugData->location,
nativeLocationOf(identifier),
ssl,
"This declaration shadows a declaration outside the inline assembly block."
);
Expand Down
40 changes: 20 additions & 20 deletions libsolidity/analysis/TypeChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
solAssert(var->type(), "Expected variable type!");
if (var->immutable())
{
m_errorReporter.typeError(3773_error, _identifier.debugData->location, "Assembly access to immutable variables is not supported.");
m_errorReporter.typeError(3773_error, nativeLocationOf(_identifier), "Assembly access to immutable variables is not supported.");
return false;
}
if (var->isConstant())
Expand All @@ -781,7 +781,7 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
{
m_errorReporter.typeError(
3558_error,
_identifier.debugData->location,
nativeLocationOf(_identifier),
"Constant variable is circular."
);
return false;
Expand All @@ -791,24 +791,24 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)

if (var && !var->value())
{
m_errorReporter.typeError(3224_error, _identifier.debugData->location, "Constant has no value.");
m_errorReporter.typeError(3224_error, nativeLocationOf(_identifier), "Constant has no value.");
return false;
}
else if (_context == yul::IdentifierContext::LValue)
{
m_errorReporter.typeError(6252_error, _identifier.debugData->location, "Constant variables cannot be assigned to.");
m_errorReporter.typeError(6252_error, nativeLocationOf(_identifier), "Constant variables cannot be assigned to.");
return false;
}
else if (identifierInfo.suffix == "slot" || identifierInfo.suffix == "offset")
{
m_errorReporter.typeError(6617_error, _identifier.debugData->location, "The suffixes .offset and .slot can only be used on non-constant storage variables.");
m_errorReporter.typeError(6617_error, nativeLocationOf(_identifier), "The suffixes .offset and .slot can only be used on non-constant storage variables.");
return false;
}
else if (var && var->value() && !var->value()->annotation().type && !dynamic_cast<Literal const*>(var->value().get()))
{
m_errorReporter.typeError(
2249_error,
_identifier.debugData->location,
nativeLocationOf(_identifier),
"Constant variables with non-literal values cannot be forward referenced from inline assembly."
);
return false;
Expand All @@ -818,7 +818,7 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
type(*var->value())->category() != Type::Category::RationalNumber
))
{
m_errorReporter.typeError(7615_error, _identifier.debugData->location, "Only direct number constants and references to such constants are supported by inline assembly.");
m_errorReporter.typeError(7615_error, nativeLocationOf(_identifier), "Only direct number constants and references to such constants are supported by inline assembly.");
return false;
}
}
Expand All @@ -833,19 +833,19 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
{
if (suffix != "slot" && suffix != "offset")
{
m_errorReporter.typeError(4656_error, _identifier.debugData->location, "State variables only support \".slot\" and \".offset\".");
m_errorReporter.typeError(4656_error, nativeLocationOf(_identifier), "State variables only support \".slot\" and \".offset\".");
return false;
}
else if (_context == yul::IdentifierContext::LValue)
{
if (var->isStateVariable())
{
m_errorReporter.typeError(4713_error, _identifier.debugData->location, "State variables cannot be assigned to - you have to use \"sstore()\".");
m_errorReporter.typeError(4713_error, nativeLocationOf(_identifier), "State variables cannot be assigned to - you have to use \"sstore()\".");
return false;
}
else if (suffix != "slot")
{
m_errorReporter.typeError(9739_error, _identifier.debugData->location, "Only .slot can be assigned to.");
m_errorReporter.typeError(9739_error, nativeLocationOf(_identifier), "Only .slot can be assigned to.");
return false;
}
}
Expand All @@ -857,28 +857,28 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
{
if (suffix != "offset" && suffix != "length")
{
m_errorReporter.typeError(1536_error, _identifier.debugData->location, "Calldata variables only support \".offset\" and \".length\".");
m_errorReporter.typeError(1536_error, nativeLocationOf(_identifier), "Calldata variables only support \".offset\" and \".length\".");
return false;
}
}
else
{
m_errorReporter.typeError(3622_error, _identifier.debugData->location, "The suffix \"." + suffix + "\" is not supported by this variable or type.");
m_errorReporter.typeError(3622_error, nativeLocationOf(_identifier), "The suffix \"." + suffix + "\" is not supported by this variable or type.");
return false;
}
}
else if (!var->isConstant() && var->isStateVariable())
{
m_errorReporter.typeError(
1408_error,
_identifier.debugData->location,
nativeLocationOf(_identifier),
"Only local variables are supported. To access storage variables, use the \".slot\" and \".offset\" suffixes."
);
return false;
}
else if (var->type()->dataStoredIn(DataLocation::Storage))
{
m_errorReporter.typeError(9068_error, _identifier.debugData->location, "You have to use the \".slot\" or \".offset\" suffix to access storage reference variables.");
m_errorReporter.typeError(9068_error, nativeLocationOf(_identifier), "You have to use the \".slot\" or \".offset\" suffix to access storage reference variables.");
return false;
}
else if (var->type()->sizeOnStack() != 1)
Expand All @@ -887,26 +887,26 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
auto const* arrayType = dynamic_cast<ArrayType const*>(var->type());
arrayType && arrayType->isDynamicallySized() && arrayType->dataStoredIn(DataLocation::CallData)
)
m_errorReporter.typeError(1397_error, _identifier.debugData->location, "Call data elements cannot be accessed directly. Use \".offset\" and \".length\" to access the calldata offset and length of this array and then use \"calldatacopy\".");
m_errorReporter.typeError(1397_error, nativeLocationOf(_identifier), "Call data elements cannot be accessed directly. Use \".offset\" and \".length\" to access the calldata offset and length of this array and then use \"calldatacopy\".");
else
{
solAssert(!var->type()->dataStoredIn(DataLocation::CallData), "");
m_errorReporter.typeError(9857_error, _identifier.debugData->location, "Only types that use one stack slot are supported.");
m_errorReporter.typeError(9857_error, nativeLocationOf(_identifier), "Only types that use one stack slot are supported.");
}
return false;
}
}
else if (!identifierInfo.suffix.empty())
{
m_errorReporter.typeError(7944_error, _identifier.debugData->location, "The suffixes \".offset\", \".slot\" and \".length\" can only be used with variables.");
m_errorReporter.typeError(7944_error, nativeLocationOf(_identifier), "The suffixes \".offset\", \".slot\" and \".length\" can only be used with variables.");
return false;
}
else if (_context == yul::IdentifierContext::LValue)
{
if (dynamic_cast<MagicVariableDeclaration const*>(declaration))
return false;

m_errorReporter.typeError(1990_error, _identifier.debugData->location, "Only local variables can be assigned to in inline assembly.");
m_errorReporter.typeError(1990_error, nativeLocationOf(_identifier), "Only local variables can be assigned to in inline assembly.");
return false;
}

Expand All @@ -915,7 +915,7 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
solAssert(!!declaration->type(), "Type of declaration required but not yet determined.");
if (dynamic_cast<FunctionDefinition const*>(declaration))
{
m_errorReporter.declarationError(2025_error, _identifier.debugData->location, "Access to functions is not allowed in inline assembly.");
m_errorReporter.declarationError(2025_error, nativeLocationOf(_identifier), "Access to functions is not allowed in inline assembly.");
return false;
}
else if (dynamic_cast<VariableDeclaration const*>(declaration))
Expand All @@ -925,7 +925,7 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
{
if (!contract->isLibrary())
{
m_errorReporter.typeError(4977_error, _identifier.debugData->location, "Expected a library.");
m_errorReporter.typeError(4977_error, nativeLocationOf(_identifier), "Expected a library.");
return false;
}
}
Expand Down
2 changes: 1 addition & 1 deletion libsolidity/analysis/ViewPureChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class AssemblyViewPureChecker
if (yul::EVMDialect const* dialect = dynamic_cast<decltype(dialect)>(&m_dialect))
if (yul::BuiltinFunctionForEVM const* fun = dialect->builtin(_funCall.functionName.name))
if (fun->instruction)
checkInstruction(_funCall.debugData->location, *fun->instruction);
checkInstruction(nativeLocationOf(_funCall), *fun->instruction);

for (auto const& arg: _funCall.arguments)
std::visit(*this, arg);
Expand Down
4 changes: 2 additions & 2 deletions libsolidity/ast/ASTJsonConverter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,10 @@ void ASTJsonConverter::appendExpressionAttributes(
_attributes += exprAttributes;
}

Json::Value ASTJsonConverter::inlineAssemblyIdentifierToJson(pair<yul::Identifier const* ,InlineAssemblyAnnotation::ExternalIdentifierInfo> _info) const
Json::Value ASTJsonConverter::inlineAssemblyIdentifierToJson(pair<yul::Identifier const*, InlineAssemblyAnnotation::ExternalIdentifierInfo> _info) const
{
Json::Value tuple(Json::objectValue);
tuple["src"] = sourceLocationToString(_info.first->debugData->location);
tuple["src"] = sourceLocationToString(nativeLocationOf(*_info.first));
Comment on lines -174 to +177
Copy link
Collaborator Author

@cameel cameel Sep 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing this to originLocationOf() does not break any tests so but I guess this is because we cannot have debug info annotations in inline assembly?

tuple["declaration"] = idOrNull(_info.second.declaration);
tuple["isSlot"] = Json::Value(_info.second.suffix == "slot");
tuple["isOffset"] = Json::Value(_info.second.suffix == "offset");
Expand Down
2 changes: 1 addition & 1 deletion libsolidity/codegen/CompilerContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ void CompilerContext::appendInlineAssembly(
if (stackDiff < 1 || stackDiff > 16)
BOOST_THROW_EXCEPTION(
StackTooDeepError() <<
errinfo_sourceLocation(_identifier.debugData->location) <<
errinfo_sourceLocation(nativeLocationOf(_identifier)) <<
util::errinfo_comment("Stack too deep (" + to_string(stackDiff) + "), try removing local variables.")
);
if (_context == yul::IdentifierContext::RValue)
Expand Down
2 changes: 1 addition & 1 deletion libsolidity/parsing/Parser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1326,7 +1326,7 @@ ASTPointer<InlineAssembly> Parser::parseInlineAssembly(ASTPointer<ASTString> con
if (block == nullptr)
BOOST_THROW_EXCEPTION(FatalError());

location.end = block->debugData->location.end;
location.end = nativeLocationOf(*block).end;
return make_shared<InlineAssembly>(nextID(), location, _docString, dialect, block);
}

Expand Down
48 changes: 37 additions & 11 deletions libyul/AST.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,34 @@ using Type = YulString;

struct DebugData
{
explicit DebugData(langutil::SourceLocation _location, std::optional<int64_t> _astID = {}):
location(std::move(_location)),
explicit DebugData(
langutil::SourceLocation _nativeLocation,
langutil::SourceLocation _originLocation = {},
std::optional<int64_t> _astID = {}
):
nativeLocation(std::move(_nativeLocation)),
originLocation(std::move(_originLocation)),
astID(std::move(_astID))
{}

static std::shared_ptr<DebugData const> create(
langutil::SourceLocation _location = {},
langutil::SourceLocation _nativeLocation = {},
langutil::SourceLocation _originLocation = {},
std::optional<int64_t> _astID = {}
)
{
return std::make_shared<DebugData const>(std::move(_location), std::move(_astID));
return std::make_shared<DebugData const>(
std::move(_nativeLocation),
std::move(_originLocation),
std::move(_astID)
);
}

langutil::SourceLocation location;
/// Location in the Yul code.
langutil::SourceLocation nativeLocation;
/// Location in the original source that the Yul code was produced from.
/// Optional. Only present if the Yul source contains location annotations.
langutil::SourceLocation originLocation;
/// ID in the (Solidity) source AST.
std::optional<int64_t> astID;
};
Expand Down Expand Up @@ -94,16 +108,28 @@ struct Continue { std::shared_ptr<DebugData const> debugData; };
/// Leave statement (valid within function)
struct Leave { std::shared_ptr<DebugData const> debugData; };

/// Extracts the source location from a Yul node.
template <class T> inline langutil::SourceLocation locationOf(T const& _node)
/// Extracts the IR source location from a Yul node.
template <class T> inline langutil::SourceLocation nativeLocationOf(T const& _node)
{
return _node.debugData ? _node.debugData->nativeLocation : langutil::SourceLocation{};
}

/// Extracts the IR source location from a Yul node.
template <class... Args> inline langutil::SourceLocation nativeLocationOf(std::variant<Args...> const& _node)
{
return std::visit([](auto const& _arg) { return nativeLocationOf(_arg); }, _node);
}

/// Extracts the original source location from a Yul node.
template <class T> inline langutil::SourceLocation originLocationOf(T const& _node)
{
return _node.debugData ? _node.debugData->location : langutil::SourceLocation{};
return _node.debugData ? _node.debugData->originLocation : langutil::SourceLocation{};
}

/// Extracts the source location from a Yul node.
template <class... Args> inline langutil::SourceLocation locationOf(std::variant<Args...> const& _node)
/// Extracts the original source location from a Yul node.
template <class... Args> inline langutil::SourceLocation originLocationOf(std::variant<Args...> const& _node)
{
return std::visit([](auto const& _arg) { return locationOf(_arg); }, _node);
return std::visit([](auto const& _arg) { return originLocationOf(_arg); }, _node);
}

/// Extracts the debug data from a Yul node.
Expand Down
Loading