Skip to content

Make legacy codegen remove custom errors when compiler option revert-strings=strip is requested #16466

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

Open
matheusaaguiar wants to merge 3 commits into
base: develop
Choose a base branch
from
+72 −9
Open

Make legacy codegen remove custom errors when compiler option revert-strings=strip is requested #16466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17c61de
Suppress custom errors when revert-strings strip is requested
matheusaaguiar Feb 11, 2026
41d868c
Changelog
matheusaaguiar Feb 11, 2026
554b04e
fixup! Changelog
matheusaaguiar Feb 11, 2026
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
1 change: 1 addition & 0 deletions Changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Compiler Features:
* Yul Optimizer: Remove redundant prerequisite steps from the default optimizer sequence.

Bugfixes:
* Code Generator: Remove custom error arguments of `require` when option `revert-strings=strip` is selected in the legacy pipeline.


### 0.8.33 (2025-12-18)
Expand Down
23 changes: 14 additions & 9 deletions libsolidity/codegen/ExpressionCompiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1298,15 +1298,20 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
m_context << Instruction::ISZERO << Instruction::ISZERO;
AssemblyItem successBranchTag = m_context.appendConditionalJump();

auto const* errorDefinition = dynamic_cast<ErrorDefinition const*>(ASTNode::referencedDeclaration(errorConstructorCall.expression()));
solAssert(errorDefinition && errorDefinition->functionType(true));
utils().revertWithError(
errorDefinition->functionType(true)->externalSignature(),
errorDefinition->functionType(true)->parameterTypes(),
errorConstructorArgumentTypes
);
// Here, the argument is consumed, but in the other branch, it is still there.
m_context.adjustStackOffset(static_cast<int>(sizeOfErrorArguments));
if (m_context.revertStrings() == RevertStrings::Strip)
m_context.appendRevert();
Copy link
Contributor

@nikola-matic nikola-matic Feb 11, 2026

Choose a reason for hiding this comment

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

My suspicion is that you'll leave the stack dirty here (i.e. when we iterate and handle the constructor call arguments (loop at 1279), we'll also push any non literal arguments to the stack, which will not be popped anywhere.

Take a look at the revert with error(string) implementation.

else
{
auto const* errorDefinition = dynamic_cast<ErrorDefinition const*>(ASTNode::referencedDeclaration(errorConstructorCall.expression()));
solAssert(errorDefinition && errorDefinition->functionType(true));
utils().revertWithError(
errorDefinition->functionType(true)->externalSignature(),
errorDefinition->functionType(true)->parameterTypes(),
errorConstructorArgumentTypes
);
// Here, the argument is consumed, but in the other branch, it is still there.
m_context.adjustStackOffset(static_cast<int>(sizeOfErrorArguments));
}
m_context << successBranchTag;
// In case of the success branch i.e. require(true, ...), pop error constructor arguments
utils().popStackSlots(sizeOfErrorArguments);
Expand Down
1 change: 1 addition & 0 deletions test/cmdlineTests/revert_strings_strip_legacy/args
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--revert-strings strip --debug-info none --asm --optimize
8 changes: 8 additions & 0 deletions test/cmdlineTests/revert_strings_strip_legacy/input.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
pragma solidity >=0.0;
// SPDX-License-Identifier: GPL-3.0
contract C {
error MyError(string errorMsg);
Copy link
Contributor

@nikola-matic nikola-matic Feb 11, 2026

Choose a reason for hiding this comment

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

I'd give this an argument or two more, e.g. error MyError(string errorMsg, uint256 errorId);

function f() pure external {
require(false, MyError("error"));
Copy link
Contributor

@nikola-matic nikola-matic Feb 11, 2026

Choose a reason for hiding this comment

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

For the callsite, declare variables, assign them values, and pass them as arguments to the error constructor call.

string errorMsg = "error";
uint256 errorId = 111;

require(false, MyError(errorMsg, errorId));

I wanna see some pushes to the stack.

}
}
48 changes: 48 additions & 0 deletions test/cmdlineTests/revert_strings_strip_legacy/output
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@

======= input.sol:C =======
EVM assembly:
mstore(0x40, 0x80)
callvalue
dup1
iszero
tag_1
jumpi
revert(0x00, 0x00)
tag_1:
pop
dataSize(sub_0)
dup1
dataOffset(sub_0)
0x00
codecopy
0x00
return
stop

sub_0: assembly {
mstore(0x40, 0x80)
callvalue
dup1
iszero
tag_1
jumpi
revert(0x00, 0x00)
tag_1:
pop
jumpi(tag_2, lt(calldatasize, 0x04))
shr(0xe0, calldataload(0x00))
dup1
0x26121ff0
eq
tag_3
jumpi
tag_2:
revert(0x00, 0x00)
tag_3:
tag_4
revert(0x00, 0x00)
tag_4:
stop

auxdata: <AUXDATA REMOVED>
}