Skip to content

Do not remove custom errors in IR codegen when strip revert strings 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
Open

Do not remove custom errors in IR codegen when strip revert strings 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
5839c0b
Add tests
matheusaaguiar Feb 24, 2026
78a64fb
Do not remove custom errors in IR pipeline when strip revert strings …
matheusaaguiar Feb 26, 2026
bd6acbd
Update docs
matheusaaguiar Mar 27, 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 @@ -17,6 +17,7 @@ Compiler Features:
Bugfixes:
* Yul: Fix incorrect serialization of Yul object names containing double quotes and escape sequences, producing output that could not be parsed as valid Yul.
* Yul EVM Code Transform: Improve stack shuffler performance by fixing a BFS deduplication issue.
* Yul IR Code Generation: Preserve custom error argument of `require` when option `revert-strings=strip` is selected.


### 0.8.34 (2026-02-18)
Expand Down
4 changes: 3 additions & 1 deletion docs/using-the-compiler.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,9 @@ Input Description
// How to treat revert (and require) reason strings. Settings are
// "default", "strip", "debug" and "verboseDebug".
// "default" does not inject compiler-generated revert strings and keeps user-supplied ones.
// "strip" removes all revert strings (if possible, i.e. if literals are used) keeping side-effects
// "strip" removes all revert strings (if possible, i.e. if literals are used) keeping side-effects.
// NOTE: "strip" does not remove custom errors.
// WARNING: Before Solidity 0.8.35, "strip" in the IR pipeline also removed custom errors.
// "debug" injects strings for compiler-generated internal reverts, implemented for ABI encoders V1 and V2 for now.
// "verboseDebug" even appends further information to user-supplied revert strings (not yet implemented)
"revertStrings": "default",
Expand Down
8 changes: 4 additions & 4 deletions libsolidity/codegen/ir/IRGeneratorForStatements.cpp
View file
Open in desktop
Copy link
Copy Markdown
Contributor

@rodiazet rodiazet Mar 17, 2026

Choose a reason for hiding this comment

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

  1. Would also add a test case, in the same contract, with stripped plain string message and un-stripped custom error message,
  2. require_error_evaluation_order_2.sol tests proper order of require arguments evaluation. Does it also test this in case of revertStrings: Strip? Probably not.
  3. Similar require_inherited_error.sol also is tested only for non-strip version.
  4. Same for require_error_condition_evaluated_only_once.sol it's only tested for non-strip version, and it should be tested because it old code had different control flow.

Copy link
Copy Markdown
Contributor Author

@matheusaaguiar matheusaaguiar Mar 29, 2026

Choose a reason for hiding this comment

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

Added

Original file line number Diff line number Diff line change
Expand Up @@ -1158,10 +1158,7 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall)
solAssert(arguments.size() > 0, "Expected at least one parameter for require/assert");
solAssert(arguments.size() <= 2, "Expected no more than two parameters for require/assert");

Type const* messageArgumentType =
arguments.size() > 1 && m_context.revertStrings() != RevertStrings::Strip ?
arguments[1]->annotation().type :
nullptr;
Type const* messageArgumentType = arguments.size() == 2 ? arguments[1]->annotation().type : nullptr;

auto const* magicType = dynamic_cast<MagicType const*>(messageArgumentType);
if (magicType && magicType->kind() == MagicType::Kind::Error)
Copy link
Copy Markdown
Contributor

@rodiazet rodiazet Mar 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

In the new version this fragment of the code also generates code for path when the require conditions evaluates to true, but I cannot find tests for this case except require_error_stack_check.sol. Not sure if it tests revertStrings = Strip. IMO we should also add test which does not revert and check that the stack is properly cleaned and not revert is called.
BTW in previous version with Strip, custom Error was handled by the code in the else branch. Was it correct?

Copy link
Copy Markdown
Contributor Author

@matheusaaguiar matheusaaguiar Mar 19, 2026

Choose a reason for hiding this comment

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

In the new version this fragment of the code also generates code for path when the require conditions evaluates to true, but I cannot find tests for this case except require_error_stack_check.sol. Not sure if it tests revertStrings = Strip.

The new version now follows the same path of previous version when revertStrings != Strip, so it should have the same result. I will add a variation of that test with revert-strings=string anyway.

BTW in previous version with Strip, custom Error was handled by the code in the else branch. Was it correct?

The handling was the same of a require with no message, i.e., YulUtilFunctions::requireOrAssertFunction will check if the message type is null and then generate code with revert(0, 0) for the false branch of the require condition.
The arguments of require are already visited and their code generated by this point (side-effects guaranteed to happen).
So I think it was correct.

Expand All @@ -1175,6 +1172,9 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall)
}
else
{
// This option only removes strings, not custom errors
if (m_context.revertStrings() == RevertStrings::Strip)
messageArgumentType = nullptr;
ASTPointer<Expression const> stringArgumentExpression = messageArgumentType ? arguments[1] : nullptr;
std::string requireOrAssertFunction = m_utils.requireOrAssertFunction(
functionType->kind() == FunctionType::Kind::Assert,
Expand Down
1 change: 1 addition & 0 deletions test/cmdlineTests/revert_strings_strip_legacy_require_custom_error/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
11 changes: 11 additions & 0 deletions test/cmdlineTests/revert_strings_strip_legacy_require_custom_error/input.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
pragma solidity >=0.0;
// SPDX-License-Identifier: GPL-3.0
contract C {
error MyError(uint errorCode, string errorMsg, bool flag);
function flag() pure public returns (bool) { return false; }
function f() pure external {
uint code = 8;
string memory eMsg = "error";
require(false, MyError(code, eMsg, flag()));
}
}
173 changes: 173 additions & 0 deletions test/cmdlineTests/revert_strings_strip_legacy_require_custom_error/output
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@

======= 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
dup1
0x890eba68
eq
tag_4
jumpi
tag_2:
revert(0x00, 0x00)
tag_3:
tag_5
tag_6
jump // in
tag_5:
stop
tag_4:
0x40
dup1
mload
0x00
dup2
mstore
swap1
mload
swap1
dup2
swap1
sub
0x20
add
swap1
return
tag_6:
0x40
dup1
mload
dup1
dup3
add
swap1
swap2
mstore
0x05
dup2
mstore
shl(0xd9, 0x32b93937b9)
0x20
dup3
add
mstore
0x08
swap1
dup2
dup2
0x00
mload(0x40)
shl(0xe5, 0x01676c8b)
dup2
mstore
0x04
add
tag_14
swap4
swap3
swap2
swap1
tag_15
jump // in
tag_14:
mload(0x40)
dup1
swap2
sub
swap1
revert
tag_15:
dup4
dup2
mstore
0x60
0x20
dup3
add
mstore
0x00
dup4
mload
dup1
0x60
dup5
add
mstore
dup1
0x20
dup7
add
0x80
dup6
add
mcopy
0x00
0x80
dup3
dup6
add
add
mstore
0x80
0x1f
not
0x1f
dup4
add
and
dup5
add
add
swap2
pop
pop
dup3
iszero
iszero
0x40
dup4
add
mstore
swap5
swap4
pop
pop
pop
pop
jump // out

auxdata: <AUXDATA REMOVED>
}
1 change: 1 addition & 0 deletions ...Tests/revert_strings_strip_legacy_require_custom_error_constructor_param_side_effect/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
12 changes: 12 additions & 0 deletions .../revert_strings_strip_legacy_require_custom_error_constructor_param_side_effect/input.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
pragma solidity >=0.0;
// SPDX-License-Identifier: GPL-3.0
contract C {
error MyError(uint errorCode, string errorMsg);
uint public counter = 0;
function count() public returns (uint) { return ++counter; }
function f() external {
string memory eMsg = "error";
require(false, MyError(count(), eMsg));
counter += 2;
}
}
Loading