Skip to content

Commit

Permalink
Merge pull request #2675 from aeternity/aevm-nested-map-bug
Browse files Browse the repository at this point in the history
AEVM nested map bug
  • Loading branch information
UlfNorell committed Aug 21, 2019
2 parents b4d1956 + 01b67aa commit a471e33
Show file tree
Hide file tree
Showing 9 changed files with 553 additions and 28 deletions.
39 changes: 27 additions & 12 deletions apps/aecontract/test/aecontract_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -195,12 +195,12 @@
-define(qid(__X__), {'@oq', __X__}).


-define(assertMatchAEVM(Res, ExpVm1, ExpVm2, ExpVm3),
-define(assertMatchAEVM(Res, ExpVm1, ExpVm2, ExpVm3, ExpVm4),
case vm_version() of
?VM_AEVM_SOPHIA_1 -> ?assertMatch(ExpVm1, Res);
?VM_AEVM_SOPHIA_2 -> ?assertMatch(ExpVm2, Res);
?VM_AEVM_SOPHIA_3 -> ?assertMatch(ExpVm3, Res);
?VM_AEVM_SOPHIA_4 -> ?assertMatch(ExpVm3, Res); %% So far no difference
?VM_AEVM_SOPHIA_4 -> ?assertMatch(ExpVm4, Res);
?VM_FATE_SOPHIA_1 -> ok
end).

Expand Down Expand Up @@ -343,6 +343,7 @@ groups() ->
sophia_maps,
sophia_map_benchmark,
sophia_big_map_benchmark,
sophia_map_of_maps,
sophia_variant_types,
sophia_arity_check,
sophia_chain,
Expand Down Expand Up @@ -1787,6 +1788,7 @@ sophia_call_origin(_Cfg) ->
RemCInt,
%% After Roma, the Call.caller and Call.origin is NOT the same.
AccInt,
AccInt,
AccInt),
?assertMatchFATE({address, AccInt}, Origin2),
Caller2 = ?call(call_contract, Acc, RemC, nested_caller, word, {}),
Expand Down Expand Up @@ -1837,7 +1839,7 @@ sophia_oracles(_Cfg) ->
?assertMatchAEVM(
?call(call_contract, Acc, Ct, queryFee, word, BogusOracle),
32, %% On ROMA this is broken, returns 32.
{error, _}, {error, _}), %% Fixed in MINERVA
{error, _}, {error, _}, {error, _}), %% Fixed in MINERVA
none = ?call(call_contract, Acc, Ct, getAnswer, {option, word}, {Oracle, QId}),
{} = ?call(call_contract, Acc, Ct, respond, {tuple, []}, {Oracle, QId, 4001}),
NonceAfterRespond = aec_accounts:nonce(aect_test_utils:get_account(Ct, state())),
Expand Down Expand Up @@ -3709,7 +3711,7 @@ sophia_maps(_Cfg) ->
Actual = {Call(size_state_i, word, {}),
Call(size_state_s, word, {})},
%% Maps.size was broken i ROMA, fixed in Minerva
?assertMatchAEVM(Actual, Expect1, Expect2, Expect2)
?assertMatchAEVM(Actual, Expect1, Expect2, Expect2, Expect2)
end,

%% Reset the state
Expand Down Expand Up @@ -4055,22 +4057,35 @@ sophia_map_of_maps(_Cfg) ->
{Ct, _Gas} = ?call(create_contract, Acc, map_of_maps, {}, #{gas => 1000000, return_gas_used => true}),
%% {} = ?call(call_contract, Acc, Ct, setup_state, {tuple, []}, {}),

RunTest = fun(I, Type, Expect) ->
RunTest = fun(I, Type) ->
Fun = fun(Tag) -> list_to_atom(lists:concat([test, I, "_", Tag])) end,
{} = ?call(call_contract, Acc, Ct, Fun(setup), {tuple, []}, {}),
{} = ?call(call_contract, Acc, Ct, Fun(execute), {tuple, []}, {}),
Actual = ?call(call_contract, Acc, Ct, Fun(check), Type, {}),
?assertMatch({I, X, X}, {I, Actual, Expect})
?call(call_contract, Acc, Ct, Fun(check), Type, {})
end,
Test = fun(I, Type, Expect) ->
Actual = RunTest(I, Type),
?assertMatch({I, X, X}, {I, Actual, Expect})
end,

%% Test 1 - garbage collection of inner map when outer map is garbage collected
RunTest(1, {map, string, string}, #{}),
Test(1, {map, string, string}, #{}),

%% Test 2 - ...
RunTest(2, {map, string, string}, #{<<"key">> => <<"val">>, <<"key2">> => <<"val2">>}),
Test(2, {map, string, string}, #{<<"key">> => <<"val">>, <<"key2">> => <<"val2">>}),

%% Test 3
RunTest(3, bool, true),
Test(3, bool, true),

%% Test 4 -- Broken pre VM_AEVM_SOPHIA_4

Res4 = RunTest(4, {map, string, {map, string, string}}),
Good = #{<<"a">> => #{<<"b">> => <<"c">>}},
?assertMatchAEVM(Res4, {'EXIT', {badmatch, _}, _},
{'EXIT', {badmatch, _}, _},
{'EXIT', {badmatch, _}, _},
Good),
?assertMatchFATE(Res4, Good),

ok.

Expand Down Expand Up @@ -4693,7 +4708,7 @@ sophia_heap_to_heap_bug(_Cfg) ->
?assertMatchAEVM(
?call(call_contract, Acc, IdC, f, word, {100}, #{return_gas_used => true}),
{{error,<<"out_of_gas">>}, _Gas}, %% Bad size check kicks in
{1, _Gas}, {1, _Gas}), %% But works on new VM.
{1, _Gas}, {1, _Gas}, {1, _Gas}), %% But works on new VM.

ok.

Expand Down Expand Up @@ -4834,7 +4849,7 @@ sophia_too_little_gas_for_mem(_Cfg) ->
Res = ?call(call_contract, Acc, IdC, tick, word, {}, #{gas => GasUsed - 1}),
?assertMatchAEVM(Res, {error,{throw,{aevm_eval_error,out_of_gas,0}}},
{error,{throw,{aevm_eval_error,out_of_gas,0}}},
{error,<<"out_of_gas">>}).
{error,<<"out_of_gas">>}, {error, <<"out_of_gas">>}).


%% The crowd funding example.
Expand Down
16 changes: 8 additions & 8 deletions apps/aevm/src/aevm_ae_primops.erl
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,10 @@ types(?PRIM_CALL_AENS_CLAIM,_HeapValue,_Store,_State) ->
{[word, string, word, sign_t()], tuple0_t()};
types(?PRIM_CALL_AENS_PRECLAIM,_HeapValue,_Store,_State) ->
{[word, word, sign_t()], tuple0_t()};
types(?PRIM_CALL_AENS_RESOLVE, HeapValue, Store,_State) ->
types(?PRIM_CALL_AENS_RESOLVE, HeapValue, Store, State) ->
%% The out type is given in the third argument
T = {tuple, [word, word, word, typerep]},
{ok, Bin} = aevm_data:heap_to_binary(T, Store, HeapValue),
{ok, Bin} = aevm_eeevm_state:heap_value_to_binary(T, HeapValue, State),
{ok, {_Prim, _, _, OutType}} = aeb_heap:from_binary(T, Bin),
{[string, string, typerep], option_t(OutType)};
types(?PRIM_CALL_AENS_REVOKE,_HeapValue,_Store, State) ->
Expand All @@ -162,19 +162,19 @@ types(?PRIM_CALL_MAP_DELETE,_HeapValue,_Store,_State) ->
{[word, word], word};
types(?PRIM_CALL_MAP_EMPTY,_HeapValue,_Store,_State) ->
{[typerep, typerep], word};
types(?PRIM_CALL_MAP_GET, HeapValue, Store, State) ->
types(?PRIM_CALL_MAP_GET, HeapValue, _Store, State) ->
T = {tuple, [word, word]},
{ok, Bin} = aevm_data:heap_to_binary(T, Store, HeapValue),
{ok, Bin} = aevm_eeevm_state:heap_value_to_binary(T, HeapValue, State),
{ok, {_Prim, Id}} = aeb_heap:from_binary(T, Bin),
{_KeyType, ValType} = aevm_eeevm_maps:map_type(Id, State),
{[word, word], option_t(ValType)};
types(?PRIM_CALL_MAP_PUT,_HeapValue,_Store,_State) ->
{[word, word, word], word};
types(?PRIM_CALL_MAP_SIZE,_HeapValue,_Store,_State) ->
{[word], word};
types(?PRIM_CALL_MAP_TOLIST, HeapValue, Store, State) ->
types(?PRIM_CALL_MAP_TOLIST, HeapValue, _Store, State) ->
T = {tuple, [word, word]},
{ok, Bin} = aevm_data:heap_to_binary(T, Store, HeapValue),
{ok, Bin} = aevm_eeevm_state:heap_value_to_binary(T, HeapValue, State),
{ok, {_Prim, Id}} = aeb_heap:from_binary(T, Bin),
{KeyType, ValType} = aevm_eeevm_maps:map_type(Id, State),
{[word], {list, {tuple, [KeyType, ValType]}}};
Expand Down Expand Up @@ -260,9 +260,9 @@ oracle_response_type_from_chain(HeapValue, Store, State) ->
oracle_query_type_from_chain(HeapValue, Store, State) ->
oracle_type_from_chain(HeapValue, Store, State, query).

oracle_type_from_chain(HeapValue, Store, State, Which) ->
oracle_type_from_chain(HeapValue, _Store, State, Which) ->
T = {tuple, [word, word]},
{ok, Bin} = aevm_data:heap_to_binary(T, Store, HeapValue),
{ok, Bin} = aevm_eeevm_state:heap_value_to_binary(T, HeapValue, State),
{ok, {_Prim, OracleID}} = aeb_heap:from_binary(T, Bin),
API = aevm_eeevm_state:chain_api(State),
ChainState = aevm_eeevm_state:chain_state(State),
Expand Down
12 changes: 8 additions & 4 deletions apps/aevm/src/aevm_data.erl
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
]).

-include_lib("aebytecode/include/aeb_heap.hrl").
-include_lib("../../aecontract/include/aecontract.hrl").

-type store() :: aect_contracts:store(). %% #{ binary() => binary() }

Expand Down Expand Up @@ -155,9 +156,7 @@ convert(Input, Output, MaxSize, Store, Visited, {list, T}, Val, Heap, BaseAddr)
_ -> convert(Input, Output, MaxSize, Store, Visited, {tuple, [T, {list, T}]}, Val, Heap, BaseAddr)
end;
convert(Input, binary, MaxSize, Store, _Visited, {map, KeyT, ValT}, MapId, Heap, BaseAddr) ->
{_NoMaps, Map0} = convert_map(Input, binary, Store, KeyT, ValT, MapId, Heap),
%% Will be a no-op if Input == binary
Map = aevm_eeevm_maps:flatten_map(Store, MapId, Map0),
{_NoMaps, Map} = convert_map(Input, binary, Store, KeyT, ValT, MapId, Heap),
KVs = maps:to_list(Map#pmap.data),
Size = maps:size(Map#pmap.data),
{RMem, FinalBase} =
Expand Down Expand Up @@ -245,7 +244,12 @@ get_map(binary, KeyT, ValT, Ptr, Heap) ->
#pmap{ key_t = KeyT, val_t = ValT, size = maps:size(Map), parent = none, data = Map }.

convert_map(Input, Output, Store, KeyT, ValT, Ptr, Heap) ->
Map = #pmap{ data = Data } = get_map(Input, KeyT, ValT, Ptr, Heap),
Map0 = get_map(Input, KeyT, ValT, Ptr, Heap),
Map = #pmap{ data = Data } =
case Output of %% Will be a no-op if Input == binary
binary -> aevm_eeevm_maps:flatten_map(Store, Ptr, Map0);
heap -> Map0
end,
{InnerMaps, Data1} =
convert_map_values(Input, Output, Store, ValT, Data, Heap),
{InnerMaps, Map#pmap{ data = Data1 }}.
Expand Down

0 comments on commit a471e33

Please sign in to comment.