diff options
| -rw-r--r-- | erts/emulator/beam/erl_bif_guard.c | 4 | ||||
| -rw-r--r-- | lib/compiler/src/beam_validator.erl | 6 | ||||
| -rw-r--r-- | lib/compiler/test/beam_validator_SUITE.erl | 36 |
3 files changed, 40 insertions, 6 deletions
diff --git a/erts/emulator/beam/erl_bif_guard.c b/erts/emulator/beam/erl_bif_guard.c index 8b47db10dd..9e15f4e399 100644 --- a/erts/emulator/beam/erl_bif_guard.c +++ b/erts/emulator/beam/erl_bif_guard.c @@ -318,6 +318,10 @@ double_to_integer(Process* p, double x) * The following code is used when a guard that may build on the * heap is called directly. They must not use HAlloc(), but must * do a garbage collection if there is insufficient heap space. + * + * Important note: All error checking MUST be done before doing + * a garbage collection. The compiler assumes that all registers + * are still valid if a guard BIF generates an exception. */ #define ERTS_NEED_GC(p, need) ((HEAP_LIMIT((p)) - HEAP_TOP((p))) <= (need)) diff --git a/lib/compiler/src/beam_validator.erl b/lib/compiler/src/beam_validator.erl index 08ba9c3ee4..5ddc534abc 100644 --- a/lib/compiler/src/beam_validator.erl +++ b/lib/compiler/src/beam_validator.erl @@ -604,9 +604,9 @@ valfun_4({gc_bif,Op,{f,Fail},Live,Src,Dst}, #vst{current=St0}=Vst0) -> St = kill_heap_allocation(St0), Vst1 = Vst0#vst{current=St}, verify_live(Live, Vst1), - Vst2 = prune_x_regs(Live, Vst1), - validate_src(Src, Vst2), - Vst = branch_state(Fail, Vst2), + Vst2 = branch_state(Fail, Vst1), + Vst = prune_x_regs(Live, Vst2), + validate_src(Src, Vst), Type = bif_type(Op, Src, Vst), set_type_reg(Type, Dst, Vst); valfun_4(return, #vst{current=#st{numy=none}}=Vst) -> diff --git a/lib/compiler/test/beam_validator_SUITE.erl b/lib/compiler/test/beam_validator_SUITE.erl index ef8feb8a27..0659d5575e 100644 --- a/lib/compiler/test/beam_validator_SUITE.erl +++ b/lib/compiler/test/beam_validator_SUITE.erl @@ -28,7 +28,7 @@ freg_range/1,freg_uninit/1,freg_state/1, bin_match/1,bin_aligned/1,bad_dsetel/1, state_after_fault_in_catch/1,no_exception_in_catch/1, - undef_label/1,illegal_instruction/1]). + undef_label/1,illegal_instruction/1,failing_gc_guard_bif/1]). -include("test_server.hrl"). @@ -52,13 +52,13 @@ all(suite) -> freg_range,freg_uninit,freg_state, bin_match,bin_aligned, bad_dsetel,state_after_fault_in_catch,no_exception_in_catch, - undef_label,illegal_instruction]. + undef_label,illegal_instruction,failing_gc_guard_bif]. beam_files(Config) when is_list(Config) -> ?line {ok,Cwd} = file:get_cwd(), ?line Parent = filename:dirname(Cwd), ?line Wc = filename:join([Parent,"*","*.beam"]), - %% Must have at least two files here, or there will could be + %% Must have at least two files here, or there will be %% a grammatical error in the output of the io:format/2 call below. ;-) ?line [_,_|_] = Fs = filelib:wildcard(Wc), ?line io:format("~p files\n", [length(Fs)]), @@ -356,6 +356,36 @@ illegal_instruction(Config) when is_list(Config) -> {{'_',y,0},{[],0,illegal_instruction}}] = Errors, ok. +%% The beam_validator used to assume that a GC guard BIF could +%% do a garbage collection even if it failed. That assumption +%% is not correct, and will cause the beam_validator to reject +%% valid programs such as this test case. +%% +%% (Thanks to Kiran Khaladkar.) +%% +failing_gc_guard_bif(Config) when is_list(Config) -> + ?line ok = process_request(lists:seq(1, 36)), + ?line error = process_request([]), + ?line error = process_request(not_a_list), + ok. + +process_request(ConfId) -> + case process_request_foo(ConfId) of + false -> + if + length(ConfId) == 36 -> + Response = ok; + true -> + Response = error + end + end, + process_request_bar(self(), [Response]). + +process_request_foo(_) -> + false. + +process_request_bar(Pid, [Response]) when is_pid(Pid) -> + Response. %%%------------------------------------------------------------------------- |