Skip to content

Use returndatacopy for retrieving dynamically sized outputs. #3308

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
chriseth merged 6 commits into from
Mar 21, 2018
Merged

Use returndatacopy for retrieving dynamically sized outputs. #3308

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2cdf44f
Move the old ABI decoder code.
chriseth Feb 20, 2018
32c94f5
Simple size check for old ABI decoder.
chriseth Dec 13, 2017
cc2f71e
Move dynamic type removal out of the type system.
chriseth Feb 20, 2018
c2709a2
Decode dynamic data.
chriseth Feb 21, 2018
cc0f702
Tests for returning dynamic data.
chriseth Feb 21, 2018
c7860a0
Changelog entry.
chriseth Mar 9, 2018
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
Prev Previous commit
Next Next commit
Move dynamic type removal out of the type system.
  • Loading branch information
@chriseth @axic
chriseth authored and axic committed Mar 21, 2018
commit cc2f71e4acede70f6c220d3d0ba407ab73c2024c
12 changes: 9 additions & 3 deletions libsolidity/analysis/TypeChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1551,16 +1551,22 @@ bool TypeChecker::visit(FunctionCall const& _functionCall)
_functionCall.expression().annotation().isPure &&
functionType->isPure();

bool allowDynamicTypes = false; // @TODO
if (!functionType)
{
m_errorReporter.typeError(_functionCall.location(), "Type is not callable");
_functionCall.annotation().type = make_shared<TupleType>();
return false;
}
else if (functionType->returnParameterTypes().size() == 1)
_functionCall.annotation().type = functionType->returnParameterTypes().front();

auto returnTypes =
allowDynamicTypes ?
functionType->returnParameterTypes() :
functionType->returnParameterTypesWithoutDynamicTypes();
if (returnTypes.size() == 1)
_functionCall.annotation().type = returnTypes.front();
else
_functionCall.annotation().type = make_shared<TupleType>(functionType->returnParameterTypes());
_functionCall.annotation().type = make_shared<TupleType>(returnTypes);

if (auto functionName = dynamic_cast<Identifier const*>(&_functionCall.expression()))
{
Expand Down
23 changes: 13 additions & 10 deletions libsolidity/ast/Types.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2311,6 +2311,18 @@ vector<string> FunctionType::parameterNames() const
return vector<string>(m_parameterNames.cbegin() + 1, m_parameterNames.cend());
}

TypePointers FunctionType::returnParameterTypesWithoutDynamicTypes() const
{
TypePointers returnParameterTypes = m_returnParameterTypes;

if (m_kind == Kind::External || m_kind == Kind::CallCode || m_kind == Kind::DelegateCall)
for (auto& param: returnParameterTypes)
if (param->isDynamicallySized() && !param->dataStoredIn(DataLocation::Storage))
Copy link
Contributor

@axic axic Mar 20, 2018

Choose a reason for hiding this comment

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

Can an external call return a storage pointer?

Copy link
Contributor Author

@chriseth chriseth Mar 20, 2018

Choose a reason for hiding this comment

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

Yes, but only via delegatecall.

Copy link
Contributor Author

@chriseth chriseth Mar 20, 2018

Choose a reason for hiding this comment

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

I.e. only a call to a library function.

param = make_shared<InaccessibleDynamicType>();

return returnParameterTypes;
}

TypePointers FunctionType::parameterTypes() const
{
if (!bound())
Expand Down Expand Up @@ -2772,18 +2784,9 @@ FunctionTypePointer FunctionType::asMemberFunction(bool _inLibrary, bool _bound)
kind = Kind::DelegateCall;
}

TypePointers returnParameterTypes = m_returnParameterTypes;
if (kind != Kind::Internal)
{
// Alter dynamic types to be non-accessible.
for (auto& param: returnParameterTypes)
if (param->isDynamicallySized())
param = make_shared<InaccessibleDynamicType>();
Copy link
Contributor

@axic axic Jan 25, 2018

Choose a reason for hiding this comment

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

We could move this check into the TypeChecker around line 1500.

Copy link
Contributor Author

@chriseth chriseth Jan 25, 2018

Choose a reason for hiding this comment

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

... only if the pragma specifies that we are using the old encoder.

}

return make_shared<FunctionType>(
parameterTypes,
returnParameterTypes,
m_returnParameterTypes,
m_parameterNames,
m_returnParameterNames,
kind,
Expand Down
3 changes: 3 additions & 0 deletions libsolidity/ast/Types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,9 @@ class FunctionType: public Type
TypePointers parameterTypes() const;
std::vector<std::string> parameterNames() const;
TypePointers const& returnParameterTypes() const { return m_returnParameterTypes; }
/// @returns the list of return parameter types. All dynamically-sized types (this excludes
/// storage pointers) are replaced by InaccessibleDynamicType instances.
TypePointers returnParameterTypesWithoutDynamicTypes() const;
std::vector<std::string> const& returnParameterNames() const { return m_returnParameterNames; }
/// @returns the "self" parameter type for a bound function
TypePointer const& selfType() const;
Expand Down
17 changes: 12 additions & 5 deletions libsolidity/codegen/ExpressionCompiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ void ExpressionCompiler::appendStateVariableAccessor(VariableDeclaration const&
utils().popStackSlots(paramTypes.size() - 1);
}
unsigned retSizeOnStack = 0;
solAssert(accessorType.returnParameterTypes().size() >= 1, "");
auto const& returnTypes = accessorType.returnParameterTypes();
auto returnTypes = accessorType.returnParameterTypes();
solAssert(returnTypes.size() >= 1, "");
if (StructType const* structType = dynamic_cast<StructType const*>(returnType.get()))
{
// remove offset
Expand Down Expand Up @@ -1618,15 +1618,22 @@ void ExpressionCompiler::appendExternalFunctionCall(
m_context.experimentalFeatureActive(ExperimentalFeature::V050) &&
m_context.evmVersion().hasStaticCall();

bool allowDynamicTypes = false; // @TODO
unsigned retSize = 0;
TypePointers returnTypes;
if (returnSuccessCondition)
retSize = 0; // return value actually is success condition
else if (allowDynamicTypes)
returnTypes = _functionType.returnParameterTypes();
else
for (auto const& retType: _functionType.returnParameterTypes())
{
returnTypes = _functionType.returnParameterTypesWithoutDynamicTypes();
for (auto const& retType: returnTypes)
{
solAssert(!retType->isDynamicallySized(), "Unable to return dynamic type from external call.");
retSize += retType->calldataEncodedSize();
}
}

// Evaluate arguments.
TypePointers argumentTypes;
Expand Down Expand Up @@ -1824,11 +1831,11 @@ void ExpressionCompiler::appendExternalFunctionCall(
utils().fetchFreeMemoryPointer();
m_context << Instruction::SUB << Instruction::MLOAD;
}
else if (!_functionType.returnParameterTypes().empty())
else if (!returnTypes.empty())
{
utils().fetchFreeMemoryPointer();
bool memoryNeeded = false;
for (auto const& retType: _functionType.returnParameterTypes())
for (auto const& retType: returnTypes)
{
utils().loadFromMemoryDynamic(*retType, false, true, true);
if (dynamic_cast<ReferenceType const*>(retType.get()))
Expand Down