Skip to content

Fix ICE for user-defined value type in external library function #16509

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
chfast wants to merge 1 commit into
base: develop
Choose a base branch
from
+75 −1
Open

Fix ICE for user-defined value type in external library function #16509

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 @@ -12,6 +12,7 @@ Compiler Features:
* Yul EVM Code Transform: Improve stack shuffler performance by fixing a BFS deduplication issue.

Bugfixes:
* Type System: Fix internal compiler error when using user-defined value types in storage arrays or mappings in external library functions.
Copy link
Copy Markdown
Collaborator

@cameel cameel Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Return parameters were unaffected since they are not a part of the signature, right?

Suggested change
* Type System: Fix internal compiler error when using user-defined value types in storage arrays or mappings in external library functions.
* Type System: Fix internal compiler error when defining external library functions accepting storage pointers to arrays/mappings of user-defined value types.



### 0.8.34 (2026-02-18)
Expand Down
5 changes: 4 additions & 1 deletion libsolidity/ast/Types.h
View file
Open in desktop
Comment thread
cameel marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -1196,7 +1196,10 @@ class UserDefinedValueType: public Type

std::string toString(bool _withoutDataLocation) const override;
std::string canonicalName() const override;
std::string signatureInExternalFunction(bool) const override { solAssert(false, ""); }
std::string signatureInExternalFunction(bool _structsByName) const override
{
return underlyingType().signatureInExternalFunction(_structsByName);
}

protected:
std::vector<std::tuple<std::string, Type const*>> makeStackItems() const override;
Expand Down
70 changes: 70 additions & 0 deletions test/libsolidity/semanticTests/userDefinedValueType/storage_array_library.sol
View file
Open in desktop
Copy link
Copy Markdown
Collaborator

@cameel cameel Apr 2, 2026

Choose a reason for hiding this comment

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

In addition to a semantic test we also need one that shows what the new signatures look like so that we can evaluate how the change impacts the ABI. Especially how they differ between libraries and contracts and between memory and storage arguments. ABI has very strong backwards-compatibility guarantees, so if we get it wrong, we'll be stuck with mess that we won't be able to fix easily.

Here's one I just used to investigate this manually. It should be added to cmdlineTest/:

contract C {}
interface I {}
type U is uint;
struct S { U x; E y; C c; I i; }
enum E {A, B, C}

library LMap {
    function getUDVTMapping(mapping(U => U) storage) external {}
    function getEnumMapping(mapping(E => E) storage) external {}
    function getStructMapping(mapping(uint => S) storage) external {}
    function getContractMapping(mapping(C => I) storage) external {}
}

library LStor {
    function getUDVT(U[] storage) external {}
    function getBytes(bytes[] storage) external {}
    function getStruct(S[] storage) external {}
    function getEnum(E[] storage) external {}
    function getContract(C[] storage) external {}
    function getInterface(I[] storage) external {}
    function getFunction(function(U, S memory, E) external[] storage) external view {}
}

library LMem {
    function getUDVT(U[] memory) external {}
    function getBytes(bytes[] memory) external {}
    function getStruct(S[] memory) external {}
    function getEnum(E[] memory) external {}
    function getContract(C[] memory) external {}
    function getInterface(I[] memory) external {}
    function getFunction(function(U, S memory, E) external[] memory) external view {}
}

contract CMem {
    function getUDVT(U[] memory) external {}
    function getBytes(bytes[] memory) external {}
    function getStruct(S[] memory) external {}
    function getEnum(E[] memory) external {}
    function getContract(C[] memory) external {}
    function getInterface(I[] memory) external {}
    function getFunction(function(U, S memory, E) external[] memory) external view {}
}

Here's the relevant part of the output of solc --hashes from your branch:

======= test.sol:LMap =======
Function signatures:
027b4c0e: getContractMapping(mapping(C => I) storage)
d02c5bc5: getEnumMapping(mapping(E => E) storage)
88246425: getStructMapping(mapping(uint256 => S) storage)
5f175033: getUDVTMapping(mapping(U => U) storage)

======= test.sol:LStor =======
Function signatures:
c8a96f7b: getBytes(bytes[] storage)
4b8c90be: getContract(C[] storage)
7652387c: getEnum(E[] storage)
c738c8fb: getFunction(function[] storage)
5b6d96b0: getInterface(I[] storage)
93bb8e52: getStruct(S[] storage)
89fd932b: getUDVT(uint256[] storage)

======= test.sol:LMem =======
Function signatures:
e4b4deb7: getBytes(bytes[])
5ef1bc6a: getContract(C[])
d2c247c9: getEnum(E[])
afd7a10c: getFunction(function[])
392067e7: getInterface(I[])
be8c0fb1: getStruct(S[])
ed352282: getUDVT(uint256[])

======= test.sol:CMem =======
Function signatures:
e4b4deb7: getBytes(bytes[])
ca3cbc19: getContract(address[])
d23989a5: getEnum(uint8[])
afd7a10c: getFunction(function[])
df8e7709: getInterface(address[])
bbabad85: getStruct((uint256,uint8,address,address)[])
ed352282: getUDVT(uint256[])

Would be good to also add a similar one for scalar types (without mappings/arrays).

Copy link
Copy Markdown
Collaborator

@cameel cameel Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Looking at the output, there are some irregularities. It's odd that we decided to use the underlying type for UDVTs in libraries. We do not do this with any other user-defined type. Enums, contracts, interfaces, structs all use the user-provided name.

I wonder if this was even intentional. It contradicts what the documentation says about value types (Function Signatures and Selectors in Libraries):

Value types, non-storage string and non-storage bytes use the same identifiers as in the contract ABI.

It looks like a bug to me, but it's too late to fix it now. We have to live with it. Could you correct that in the documentation? After you do that, it would be best to explicitly mention that UDVTs behave differently.

Copy link
Copy Markdown
Collaborator

@cameel cameel Apr 2, 2026

Choose a reason for hiding this comment

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

The output also shows that your fix is incomplete. The types are not being unwrapped for mappings. That's probably because the assumption that user-provided name is used is hard-coded in multiple places. I suspect we'll need to special-case it for mappings. We should also think if there are cases other than mappings where it could happen.

Copy link
Copy Markdown
Collaborator

@cameel cameel Apr 2, 2026

Choose a reason for hiding this comment

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

Test names should be more specific so that you can find what you're looking for without having to look inside. We have so many of them that it's becoming unmanageable otherwise.

storage_array_library.sol -> external_library_function_with_user_defined_storage_parameter.sol

For completeness, the test should also cover enums, contracts and interfaces. Or it could be a separate test, but then please call this one external_library_function_with_udvt_or_struct_storage_parameter.sol.

Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Used to cause ICE in signatureInExternalFunction.
type MyUint is uint;
type MyAddr is address;
type MyBytes1 is bytes1;

struct S { MyUint x; }

library L {
function getArray(MyUint[] storage _a, uint _i) external view returns (MyUint) {
return _a[_i];
}
function getNestedArray(MyUint[][] storage _a, uint _i, uint _j) external view returns (MyUint) {
return _a[_i][_j];
}
function getMappingByValue(mapping(uint => MyAddr) storage _m, uint _k) external view returns (MyAddr) {
return _m[_k];
}
function getMappingByKey(mapping(MyUint => uint) storage _m, MyUint _k) external view returns (uint) {
return _m[_k];
}
function getStruct(S[] storage _s, uint _i) external view returns (MyUint) {
return _s[_i].x;
}
function getBytes1Array(MyBytes1[] storage _a, uint _i) external view returns (MyBytes1) {
return _a[_i];
}
}

contract C {
MyUint[] uintArr;
MyUint[][] nestedArr;
mapping(uint => MyAddr) addrMap;
mapping(MyUint => uint) keyMap;
S[] structArr;
MyBytes1[] bytes1Arr;

function testArray() public returns (uint) {
uintArr.push(MyUint.wrap(42));
return MyUint.unwrap(L.getArray(uintArr, 0));
}
function testNestedArray() public returns (uint) {
nestedArr.push();
nestedArr[0].push(MyUint.wrap(7));
return MyUint.unwrap(L.getNestedArray(nestedArr, 0, 0));
}
function testMappingByValue() public returns (address) {
addrMap[1] = MyAddr.wrap(address(0xBEEF));
return MyAddr.unwrap(L.getMappingByValue(addrMap, 1));
}
function testMappingByKey() public returns (uint) {
keyMap[MyUint.wrap(5)] = 123;
return L.getMappingByKey(keyMap, MyUint.wrap(5));
}
function testStruct() public returns (uint) {
structArr.push(S(MyUint.wrap(99)));
return MyUint.unwrap(L.getStruct(structArr, 0));
}
function testBytes1Array() public returns (bytes1) {
bytes1Arr.push(MyBytes1.wrap(0xab));
return MyBytes1.unwrap(L.getBytes1Array(bytes1Arr, 0));
}
}
// ----
// library: L
// testArray() -> 42
// testNestedArray() -> 7
// testMappingByValue() -> 0xbeef
// testMappingByKey() -> 123
// testStruct() -> 99
// testBytes1Array() -> 0xab00000000000000000000000000000000000000000000000000000000000000