| Age | Commit message (Collapse) | Author |
|---|
|
The following code would fail to compile:
decode(<<Code/integer, Bin/binary>>) ->
<<C1/integer, B1/binary>> = Bin,
case C1 of
X when X =:= 1 orelse X =:= 2 ->
Bin2 = <<>>;
_ ->
Bin2 = B1
end,
case Code of
1 -> decode(Bin2);
_ -> Bin2
end.
The error message would be:
t: function decode/1+28:
Internal consistency check failed - please report this bug.
Instruction: return
Error: {match_context,{x,0}}:
The beam_bsm pass would delay the creation of a sub-binary when it was
unsafe to do so. The culprit was the btb_follow_branch/3 function that
for performance reasons cached labels that had already been checked.
The problem was the safety of a label also depends on the contents
of the registers. Therefore, the key for caching needs to be both
the label and the register contents.
Reported-by: José Valim
|
|
Literal maps could cause dialyzer to crash when pretty printing the results.
Reported-by: Chris McGrath <[email protected]>
|
|
The following example would cause an internal consistency
failure in the compiler:
f() -> ok.
update() -> (fun f/0)#{u => 42}.
The reason is that internally, v3_core will (incorrectly)
rewrite update/0 to code similar to this:
update() ->
if
is_map(fun f/0) ->
maps:update(u, 42, fun f/0)
end.
Since funs are not allowed to be created in guards, incorrect and
unsafe code would be generated.
It is easy to fix the bug. There already is a is_valid_map_src/1
function in v3_core that tests whether the argument for the map update
operation can possibly be a valid map. A fun is represented as a
variable with a special name in Core Erlang, so it would not be
recognized as unsafe. All we'll need to do to fix the bug is to look
closer at variables to ensure they don't represent funs. That will
ensure that the code is rewritten in the correct way:
update() ->
error({badmap,fun f/0})
end.
Reported-by: Thomas Arts
|
|
The following code would crash v3_codegen:
order(From) ->
catch
if
From#{[] => sufficient} ->
saint
end.
Before explaining the crash, first some background on the stack
frame and the Y registers.
Certain instructions, most notably the 'call' instructions, clobber
all X registers. Before any such instruction, all X registers that
have values that will be used after the call must be saved to Y
registers (i.e. to the stack frame). adjust_stack/4 will be called
when X registers must be saved.
There is also another situation when X registers must be saved, namely
within a 'catch' if we are about to execute any instruction that may
cause an exception. Examples of such instructions are some guard BIFs
(such as length/1) and construction of binaries or maps. Within a
'catch', X registers must be be saved because if an exception is
thrown and catched all X registers will be destroyed. The same
adjust_stack/4 function will be called for those instructions, but
only if they occur within a 'catch'.
There is actually one more complication. If there is code in
a guard within a catch, the X registers should not be saved, because
the code in a guard never clobbers any X registers that were alive
before the guard code was entered. v3_codegen is written with the
implicit assumption that code in guards never cause anything
to be saved to Y registers.
The code for building maps and binaries would incorrectly save X
registers within a guard inside a 'catch'.
For construction of binaries, that would mean that a useless but
harmelss 'move' instruction was generated.
But for construction of maps, the saving of the Y register would not
be harmless. There would be a crash when attempting to merge #sr{}
records. #sr{} records keeps track of the contents of X and Y
registers. When two separate code paths are joined (e.g. at the end of
'case' statement), the register descriptors must be reconciled.
Basically, the register descriptors for both paths must be identical.
The #sr{} record for one path must not claim that {y,0} contains
a certain value, while another path claims that {y,0} is dead.
Thus, the crash occurs in sr_merge/2 when failing to reconcile the
Y registers.
To fix this bug this bug we will introduce a new function called
maybe_adjust_stack/5. It will save X registers on the stack only
if the code is inside a catch but not inside a guard. We will
change all existing code to use this new function when appropriate.
Reported-by: Thomas Arts
|
|
beam_bool would make the following code unsafe (which would be
reported by beam_validator):
scotland(Echo) ->
found(case Echo of
Echo when true; Echo, Echo, Echo ->
Echo;
echo ->
[]
end,
Echo = placed).
found(_, _) -> million.
Basically, beam_bool would see that the 'case' would always return
the value of Echo. Thus:
scotland(Echo) ->
found(Echo, Echo = placed).
The only problem is that beam_bool would also remove a 'move'
instruction that would save Echo to the stack. Here is the
assembly code for part of the function:
{allocate_zero,1,1}.
{move,{x,0},{y,0}}. %% Save Echo on stack.
{bif,'=:=',{f,7},[{x,0},{atom,true}],{x,1}}.
{bif,'=:=',{f,7},[{x,0},{atom,true}],{x,2}}.
{bif,'=:=',{f,7},[{x,0},{atom,true}],{x,3}}.
{bif,'and',{f,7},[{x,2},{x,3}],{x,2}}.
{bif,'and',{f,7},[{x,1},{x,2}],{x,1}}.
{jump,{f,8}}.
{label,7}.
{move,{atom,false},{x,1}}.
{label,8}.
{bif,'or',{f,6},[{atom,true},{x,1}],{x,1}}.
{test,is_eq_exact,{f,6},[{x,1},{atom,true}]}. %% Jump never taken.
{jump,{f,5}}.
{label,6}.
{test,is_eq_exact,{f,9},[{x,0},{atom,echo}]}.
{move,nil,{x,0}}.
{jump,{f,5}}.
{label,9}.
{test_heap,3,0}.
{put_tuple,2,{x,0}}.
{put,{atom,case_clause}}.
{put,{y,0}}.
{line,[{location,"t.erl",5}]}.
{call_ext,1,{extfunc,erlang,error,1}}.
{jump,{f,5}}.
{label,5}.
{test,is_eq_exact,{f,12},[{atom,placed},{y,0}]}.
beam_bool would see that the is_eq_exact test at label 8 would
always succeed. It could therefore remove most of the code before
the jump to label 5. Unfortunately it also removed the essential
move of Echo to the stack:
{allocate_zero,1,1}.
%% Instruction incorrectly removed: {move,{x,0},{y,0}}.
{jump,{f,5}}.
{label,5}.
{test,is_eq_exact,{f,12},[{atom,placed},{y,0}]}.
The root cause of the problem is that the 'move' instruction is
included in the block of 'bif' instructions before label 8.
Normally the 'move' instruction would not have been discarded,
but because the left operand to the 'or' BIF is 'true', the
entire block with 'bif' instructions are dropped.
As far as I can see, there is no gain by including 'move'
instructions in the first place. There is no way that better
code will be produced. In fact, the entire optimization can
be given up if 'move' instructions are found in the block.
Thus we can fix this bug by never including any 'move' instructions
in the block of 'bif' instructions. We can also remove all the
code that deals with 'move' instructions within blocks.
Reported-by: Thomas Arts
|
|
|
|
|
|
When the 'bin_opt_info' is given, warnings without filenames
and line numbers could sometimes be produced:
no_file: Warning: INFO: matching non-variables after
a previous clause matching a variable will prevent delayed
sub binary optimization
The reason for the missing information is that #c_alias{} records lack
location information. There are several ways to fix the problem. The
easiest seems to be to get the location information from the
code).
Noticed-by: José Valim
|
|
Fix mistakes found by 'xmllint'.
|
|
|
|
|
|
* bjorn/cuddle-with-tests: (23 commits)
rand_SUITE: Speed up basic_stats/1
base64_SUITE: Speed up roundtrip/1
lists_SUITE: Test lists:concat/2
lists_SUITE: Test lists:split/2
lists_SUITE: Add a test case for lists:prefix/2
lists_SUITE: Add hof/1 to test all high-order functions
lists_SUITE: Add test for lists:takewhile/1
lists_SUITE: Run test cases in each group in parallel
lists_SUITE: Test lists:keyreplace/4
lists_SUITE: Extend flatten/1 test to also test flatlength/1
lists_SUITE: Correct test of lists:flatten/2
id_transform_SUITE: Modernize test suite
io_proto_SUITE: Speed up determination of default shell
io_proto_SUITE: Refactor up rtnode() and friends
gen_event_SUITE: Remove unnecessary sleep calls
proc_lib: Improve coverage for crash/1
proc_lib_SUITE: Eliminate compiler warnings
io_SUITE: Add coverage/1 to completely cover io_lib_pretty
io_SUITE: Extend coverage of code for testing printable chars
io_SUITE: Speed up test for bad +pc option
...
|
|
* c-rack/fix-typo3:
Fix typo in call_last/3 spec
Fix typo
Fix typo: message to send is in x(1) not x(0)
Fix another small typo
Fix typo
|
|
Make sure that all warnings produced when compiling the
test suite contains filenames and line numbers.
|
|
Instruction get_map_elements might destroy target registers when the fail-label is taken.
Only seen for patterns with two, and only two, target registers.
Specifically: we copy one register, and then jump.
foo(A,#{a := V1, b := V2}) -> ...
foo(A,#{b := V}) -> ...
call foo(value, #{a=>whops, c=>42}).
corresponding assembler:
{test,is_map,{f,5},[{x,1}]}.
{get_map_elements,{f,7},{x,1},{list,[{atom,a},{x,1},{atom,b},{x,2}]}}.
%% if 'a' exists but not 'b' {x,1} is overwritten, jump {f,7}
{move,{integer,1},{x,0}}.
{call_only,3,{f,10}}.
{label,7}.
{get_map_elements,{f,8},{x,1},{list,[{atom,b},{x,2}]}}.
%% {x,1} (src) is read with a corrupt value
{move,{x,0},{x,1}}.
{move,{integer,2},{x,0}}.
{call_only,3,{f,10}}.
The fix is to remove 'opt_moves' pass for get_map_elements instruction
in the case of two or more destinations.
Reported-by: Valery Tikhonov
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
regressions_SUITE will have code snippets which previously crashed the compiler.
This commits includes a test for Maps crash in beam_bool.
|
|
Before beam_split the get_map_elements instruction is still in
blocks and the helper function in beam_jump did not reflect this.
Reported-by: Quviq twitter account
|
|
* bjorn/compiler/spurious-warning:
sys_core_fold: Eliminate warnings for unused terms in effect context
sys_core_fold: Eliminate warnings for unused terms
|
|
* bjorn/compiler/doc:
Update compiler documentation
(Sneaking in OTP-12769 here which is a release note for syntax_tools.
Sorry about that.)
|
|
Language cleaned up by the technical writer tmanevik from
Combitech. Proofreading and corrections by Björn Gustavsson.
|
|
|
|
* egil/opt-compile-time/OTP-12774:
stdlib: Relax erl_anno_SUITE:is_anno/1 test
Update primary bootstrap
compiler: Use Maps as type information
compiler: Use Maps instead of dict in beam_jump
compiler: Use cerl_sets instead of gb_sets in beam_type
compiler: Use Maps instead of gb_trees in beam_dead
compiler: Use cerl_sets instead of gb_sets in beam_jump
compiler: Use cerl_sets instead of sets in v3_kernel
compiler: Use cerl_sets instead of gb_sets in sys_core_fold
compiler: Add cerl_sets module
compiler: Scope uses gb_sets not gb_trees
beam_dict: Use Maps to map function name indices
beam_dict: Use Maps to map line indices
beam_dict: Use Maps to map atom indices
v3_codegen: Use Maps to map local functions
v3_life: Refactor variable db
compiler: Use lc instead of map/1 in v3_codegen
stdlib: Optimize erl_anno:is_string/1
Conflicts:
bootstrap/lib/kernel/ebin/inet_dns.beam
bootstrap/lib/stdlib/ebin/erl_anno.beam
bootstrap/lib/stdlib/ebin/erl_lint.beam
|
|
The optimization introduced in 0a0d39d351fc could cause spurious
warnings of the type: "a term is constructed, but never used".
That would happen for constructs in effect context.
To avoid those warnings, we will need to apply warning suppression
also in effect context.
|
|
* egil/fix-compiler-beam_bsm/OTP-12758:
compiler: Add tests for beam_bsm get_map_elements
compiler: Teach beam_bsm get_map_elements instruction
|
|
Using Maps as type information container speedups files like cow_http_hd.erl
by ~500ms. Previously spent ~60% of the time in orddict:store/3.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
A sets implementation based on maps.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Small speed increase for large files.
|
|
In a5d724cf240a, a debug option for running eprof on a specific
compiler pass was added. That commit added a direct call to the eprof
module in the tools application, and therefore the test case
otp_SUITE:runtime_dependencies/1 would fail because xref would find a
call to the tools application, but tools is not listed as a runtime
dependency in compiler.app.
Since the 'eprof' option is only likely to be used by compiler
maintainers, we don't want a real dependency to the tools application.
Therefore, use c:appcall/4 to hide the call to the eprof module (and
to report the error nicely if the tools application is missing).
|
