[erlang-bugs] beam_validator does not validate against circularity in constructed values

[Erik Søe Sørensen]

Björn Gustavsson wrote:
> On Mon, Jun 27, 2011 at 11:15 AM, Erik Søe Sørensen <ess@REDACTED> wrote:
>   
>> The following function passes beam_validator verification, but calls a
>> gc_bif while a tuple is not completely constructed.
>> (I take it that that could wreak havoc in the interpreter and is thus
>> illegal and technically a validator bug; I'm not completely certain of this
>> though. Apologies if this is not a bug after all.)
>>
>>   {module, beamtoying5}.  %% version = 0
>>   {exports, [{gc_in_cons,1},{gc_in_cons2,1}]}.
>>   {attributes, []}.
>>   {labels, 4}.
>>
>>   {function, gc_in_cons, 1, 2}.
>>     {label,1}.
>>       {func_info,{atom,beamtoying5},{atom,gc_in_cons},1}.
>>     {label,2}.
>>       {test_heap,3,1}.
>>       {put_tuple,2,{x,1}}.
>>       {put,{atom,dummy}}.
>>       {gc_bif,'bnot',{f,0},1,[{x,0}],{x,0}}.
>>       {put,{x,0}}.
>>       {move,{x,1},{x,0}}.
>>       return.
>>
>>     
>
> The beam_validator WILL complain if you disable optimization:
>
> $ erlc +no_postopt beamtoying5.S
> beamtoying5: function gc_in_cons/1+8:
>   Internal consistency check failed - please report this bug.
>   Instruction: {put,{x,0}}
>   Error:       {heap_overflow,{left,0},{wanted,1}}:
>
> With optimizations enabled, the code will be rewritten to:
>
> {beam_file,beamtoying5,
>            [{gc_in_cons,1,2}],
>            [{vsn,[329530243180151502074282151407467260379]}],
>            [{options,[asm,
>                       {cwd,"/home/bjorn/test"},
>                       {outdir,"/home/bjorn/test"},
>                       time]},
>             {version,"4.7.4"},
>             {time,{2011,6,27,12,42,27}},
>             {source,"/home/bjorn/test/beamtoying5.S"}],
>            [{function,gc_in_cons,1,2,
>                       [{label,1},
>                        {func_info,{atom,beamtoying5},{atom,gc_in_cons},1},
>                        {label,2},
>                        {test_heap,2,1},
>                        {put_tuple,2,{x,1}},
>                        {put,{atom,dummy}},
>                        {gc_bif,'bnot',{f,0},2,[{x,0}],{x,0}},
>                        {test_heap,1,2},
>                        {put,{x,0}},
>                        {move,{x,1},{x,0}},
>                        return]}]}
>
> This code is not safe, in a subtle way, so there really
> is a bug in the beam_validator.
>
> I might fix this bug, but it does not have very high
> priority for me, because the loader in R14B03 (and
> in R14B02, I think) will refuse to load the resulting module,
> so it will not go unnoticed.
>   

OK, thanks for the reply (and for the pointer to +no_postopt).
I forgot that this validator issue is in fact already noted in a comment 
in beam_validator.erl, so it's not news.
(In any case, I understand that this is low priority, and that the 
validator focusing on the subtle stuff makes sense as long as its task 
is to validate compiler output.)

As for the fix, part of a solution could be to set #st.h=0 on all 
instruction which might trigger GC.
I guess a full fix would introduce a field in #st which contains the 
number of tuple fields allocated but not populated, to be checked for 
zeroness at certain instructions. (Certainly "anything that might GC" 
and "any return"; could be "anything besides put".)

Regards,
Erik