Dialyzer crash on ill-typed code
RobinMorisset opened this issue · 5 comments
On master, running dialyzer on the following code:
-module(dialyzer_fail).
f(<<X:[]>>) ->
{}.causes the following crash:
dialyzer: Analysis failed with error:
{{case_clause,[]},
[{cerl,bitstr_bitsize,1,[{file,"cerl.erl"},{line,2388}]},
{dialyzer_typesig,traverse,3,[{file,"dialyzer_typesig.erl"},{line,259}]},
{dialyzer_typesig,traverse_list,4,
[{file,"dialyzer_typesig.erl"},{line,628}]},
{dialyzer_typesig,traverse,3,[{file,"dialyzer_typesig.erl"},{line,245}]},
{dialyzer_typesig,traverse_list,4,
[{file,"dialyzer_typesig.erl"},{line,628}]},
{dialyzer_typesig,handle_clauses_1,7,
[{file,"dialyzer_typesig.erl"},{line,865}]},
{dialyzer_typesig,handle_clauses,5,
[{file,"dialyzer_typesig.erl"},{line,838}]},
{dialyzer_typesig,traverse,3,[{file,"dialyzer_typesig.erl"},{line,356}]}]}
I don't know whether this is considered a bug, or whether dialyzer is allowed to crash on code like that. I'll note that erlc shows warnings on that code, but successfully compiles it.
Dialyzer should not crash, of course.
Perhaps it should give a more appropriate error message (e.g., "Phew, this Erlang code really stinks", or similar :-) )
Seriously now, in a language like Erlang there is a myriad of ways where terms with the wrong type can appear, and perhaps it is high time that the compiler does a better job in rejecting such "obviously crappy" code than just produce warnings for it -- which in turn means that all other tools which analyze source code can avoid duplicating the effort of (gracefully) handling it.
perhaps it is high time that the compiler does a better job in rejecting such "obviously crappy" code than just produce warnings for it
It would be very easy to have the compiler reject the given example. However, it would not be as easy to reject the following error:
bug6419(Bin) ->
Size = [],
case Bin of
<<X:Size>> ->
{}
end.
To find this error an optimimization pass must be run. We don't want to mandate the use of optimization passes in the Erlang compiler, and we certainly don't think it is acceptable that a compilation will succeed without optimization passes but not with them.
For consistency and ease of describing what is a valid Erlang program, we accept any term as the size.
Currently, the compiler warns for both the original example and my example. If users turn on warnings as errors, they will not miss those errors. Therefore, users will win very little if were to reject programs such as the original example.
There is another class of tools that would become more complicated: macros, code generators, language implementations, parse transforms, and test case generators. Today they can put any term into the size field of a binary segment and catch any resulting errors at run time. With the proposed change, they would have to do more careful checking to avoid causing a syntax error.
Dialyzer should not crash, of course.
I will fix.
I often wish that Erlang would finally become a serious (i.e., well-defined, consistent, etc.) language. I think this issue and its ad hoc quick fix is a perfect example of demonstrating what I mean by it.
First of all, let me clear an error. Contrary to what the issue's title claims, there is no "ill-typed code" involved here. Strictly, there is no type error (i.e., any term which is has the wrong type) anywhere. The example code involves a pattern, not any (constructed) term.
Having cleared that, let's focus on the real issue here.
According to the informal language specification:
A pattern has the same structure as a term but can contain unbound variables.
and this is what we have here: the (weird) construct <<X:[]>> used as a pattern. Note that patterns are not terms; they either match a term (in which case they bind their unbound variables), or they do not (and pattern matching proceeds to the next clause).
Unfortunately, the section about patterns in the link above, besides being informal, is extremely brief, but I would argue that an implied condition for something to be a valid pattern in (any serious definition of) Erlang is the following:
A pattern P is valid iff there exists some instantiation of P's variables that makes it a term.
In this particular case, it is (NB: even syntactically!) apparent that there does not exist any such instantiation, so this pattern is not valid in Erlang. IMO, the compiler should produce an error for this program. Note that no analysis is required here: even the Erlang parser can reject this program by insisting that the Size portion of a Bit Syntax Expression is either an integer or an expression containing (bound) variables, nothing else.
I think that the above reasoning also provides sufficient evidence why the arguments given above by @bjorng:
- However, it would not be as easy to reject the following error: ....
- For consistency and ease of describing what is a valid Erlang program, we accept any term as the size.
are very weak (if not bogus).
The first argument is weak because, as I wrote above, the <<X:[]>> pattern is (or can be) syntactically rejected by the parser, while the pattern <<X:Size>> cannot. This has nothing to do with mandating optimization passes or not.
The second argument is also quite weak, because it does not hold, currently. For example, the compiler rejects the following program:
-module(foo).
f(<<X:[]>>) when X =:= 42 ->
{}.with an illegal bit size error.
I don't get the illegal bit size error when I compile your foo module:
Eshell V13.0.1 (abort with ^G)
1> c(foo).
foo.erl:3:1: Warning: function f/1 is unused
% 3| f(<<X:[]>>) when X =:= 42 ->
% | ^
foo.erl:3:1: Warning: no clause will ever match
% 3| f(<<X:[]>>) when X =:= 42 ->
% | ^
foo.erl:3:5: Warning: this clause cannot match because '[]' is not a valid size for a binary segment
% 3| f(<<X:[]>>) when X =:= 42 ->
% | ^
foo.erl:3:7: Warning: a size expression in a pattern evaluates to a non-integer value; this pattern cannot possibly match
% 3| f(<<X:[]>>) when X =:= 42 ->
% | ^
{ok,foo}
Four warnings, but compilation succeeded.
I should have perhaps mentioned that this not on 25.x but in an older Erlang/OTP version -- the latest one that also has HiPE, which I still use (although this issue has nothing to do with HiPE).
Erlang/OTP 22 [erts-10.7.2.4] [source] [64-bit] [smp:64:64] [ds:64:64:10] [async-threads:1] [hipe] [sharing-preserving]
Eshell V10.7.2.4 (abort with ^G)
1> c(foo).
foo.erl:3: illegal bit size
foo.erl:3: Warning: function f/1 is unused
errorHas the compiler's behavior changed? (to the worse, IMO...)