File 1921-compiler-Allow-ssa_opt-to-run-on-modules-w... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 1921-compiler-Allow-ssa_opt-to-run-on-modules-with-NIFs.patch of Package erlang (Revision a95c38533dd94a5c195230b3b146d029)

Currently displaying revision a95c38533dd94a5c195230b3b146d029 , Show latest

From f191990900a0a42951af5f5100f20503e4e340d5 Mon Sep 17 00:00:00 2001
From: Frej Drejhammar <frej.drejhammar@gmail.com>
Date: Tue, 16 May 2023 15:59:06 +0200
Subject: [PATCH 1/2] compiler: Allow ssa_opt to run on modules with NIFs

This patch allows the beam_ssa_opt-pass to run on modules containing
NIFs.

NIFs are problematic for the SSA optimization passes as when a loaded
NIF replaces a function, essentially all bets are off as callers of
the NIF cannot make any assumptions on the result of calling the NIF.
Traditionally, the SSA optimization passes have completely ignored
modules containing a call to erlang:load_nif/2. A safe way to handle
NIFs, but still allow optimization of modules containing NIFs is to
make calls to NIFs look like external calls to the optimization
passes.

The function isolate_nifs/1 transforms the input #b_module{} by
rewriting all calls to NIFs in the module to to calls to external
functions with the same names in another module. As this also removes
all callers to the NIF, all non-exported NIFs are forcibly exported,
to avoid them being removed as dead code. As all passes know how
handle external calls, this allows for safe optimization of the
module. That a NIF function can contain BEAM code which calls other
functions in the module is not a problem, at worst it leads to missed
optimizations. When all sub-passes of beam_ssa_opt have been executed
restore_nifs/2 undoes the module transforms done by isolate_nifs/1.

To avoid extra book-keeping to keep track of rewritten calls for use
by restore_nifs/2, the module to which the calls are redirected is
given a name, '\nnifs', which cannot be created by the user.

As it can be assumed that the existing beam_ssa_opt sub-passes handle
external calls properly, testing of this patch is limited to checking:
that the result of a NIF is not constant propagated; that the presence
of a call to erlang:load_nif/2 does not disable optimization of
functions not calling a NIF; and that non-exported NIFs, stay
non-exported (that exported NIFs, stay exported is already covered by
the nif_SUITE tests).
---
 erts/emulator/test/nif_SUITE.erl              |  11 +-
 lib/compiler/src/beam_ssa_opt.erl             | 140 ++++++++++++++++--
 .../test/beam_ssa_check_SUITE_data/nifs.erl   |  61 ++++++++
 3 files changed, 197 insertions(+), 15 deletions(-)
 create mode 100644 lib/compiler/test/beam_ssa_check_SUITE_data/nifs.erl

diff --git a/erts/emulator/test/nif_SUITE.erl b/erts/emulator/test/nif_SUITE.erl
index 1fb9ca8933..640903151d 100644
--- a/erts/emulator/test/nif_SUITE.erl
+++ b/erts/emulator/test/nif_SUITE.erl
@@ -75,6 +75,7 @@
          nif_whereis/1, nif_whereis_parallel/1,
          nif_whereis_threaded/1, nif_whereis_proxy/1,
          nif_ioq/1,
+         non_exported_nif/1,
          match_state_arg/1,
          pid/1,
          id/1,
@@ -210,7 +211,8 @@
 suite() -> [{ct_hooks,[ts_install_cth]}].
 
 all() ->
-    [basic]
+    [basic,
+     non_exported_nif]
         ++
     [{group, G} || G <- api_groups()]
         ++
@@ -330,6 +332,13 @@ basic(Config) when is_list(Config) ->
     true = lists:member(?MODULE, erlang:system_info(taints)),
     ok.
 
+%% Check that non-exported NIFs aren't exported by the compiler's
+%% beam_ssa_opt-pass.
+non_exported_nif(Config) when is_list(Config) ->
+    ensure_lib_loaded(Config),
+    false = lists:member({lib_version,0}, ?MODULE:module_info(exports)),
+    ok.
+
 %% Test old reload feature now always fails
 reload_error(Config) when is_list(Config) ->
     TmpMem = tmpmem(),
diff --git a/lib/compiler/src/beam_ssa_opt.erl b/lib/compiler/src/beam_ssa_opt.erl
index 213b6e79a2..643b150535 100644
--- a/lib/compiler/src/beam_ssa_opt.erl
+++ b/lib/compiler/src/beam_ssa_opt.erl
@@ -49,7 +49,8 @@
 -spec module(beam_ssa:b_module(), [compile:option()]) ->
                     {'ok',beam_ssa:b_module()}.
 
-module(Module, Opts) ->
+module(Module0, Opts) ->
+    {Module,NifInfo} = isolate_nifs(Module0),
     FuncDb = case proplists:get_value(no_module_opt, Opts, false) of
                  false -> build_func_db(Module);
                  true -> #{}
@@ -69,7 +70,7 @@ module(Module, Opts) ->
               {once, Order, late_epilogue_passes(Opts)}],
 
     StMap = run_phases(Phases, StMap0, FuncDb),
-    {ok, finish(Module, StMap)}.
+    {ok, restore_nifs(finish(Module, StMap), NifInfo)}.
 
 run_phases([{module, Passes} | Phases], StMap0, FuncDb0) ->
     {StMap, FuncDb} = compile:run_sub_passes(Passes, {StMap0, FuncDb0}),
@@ -333,13 +334,7 @@ passes_1(Ps, Opts0) ->
 -spec build_func_db(#b_module{}) -> func_info_db().
 build_func_db(#b_module{body=Fs,attributes=Attr,exports=Exports0}) ->
     Exports = fdb_exports(Attr, Exports0),
-    try
-        fdb_fs(Fs, Exports, #{})
-    catch
-        %% All module-level optimizations are invalid when a NIF can override a
-        %% function, so we have to bail out.
-        throw:load_nif -> #{}
-    end.
+    fdb_fs(Fs, Exports, #{}).
 
 fdb_exports([{on_load, L} | Attrs], Exports) ->
     %% Functions marked with on_load must be treated as exported to prevent
@@ -380,11 +375,6 @@ fdb_is([#b_set{op=call,
                args=[#b_local{}=Callee | _]} | Is],
        Caller, FuncDb) ->
     fdb_is(Is, Caller, fdb_update(Caller, Callee, FuncDb));
-fdb_is([#b_set{op=call,
-               args=[#b_remote{mod=#b_literal{val=erlang},
-                               name=#b_literal{val=load_nif}},
-                     _Path, _LoadInfo]} | _Is], _Caller, _FuncDb) ->
-    throw(load_nif);
 fdb_is([#b_set{op=MakeFun,
                args=[#b_local{}=Callee | _]} | Is],
        Caller, FuncDb) when MakeFun =:= make_fun;
@@ -3532,3 +3522,125 @@ sub_arg(Old, Sub) ->
 
 new_var(Count) ->
     {#b_var{name=Count},Count+1}.
+
+%%%
+%%% NIF handling
+%%%
+%%% NIFs are problematic for the SSA optimization passes as when a
+%%% loaded NIF replaces a function, essentially all bets are off as
+%%% callers of the NIF cannot make any assumptions on the result of
+%%% calling the NIF.
+%%%
+%%% A safe way to handle NIFs, but still allow optimization of
+%%% functions not calling NIFs is to make calls to NIFs look like
+%%% external calls. For the beam_ssa_opt compiler pass this is handled
+%%% by the functions isolate_nifs/1 and restore_nifs/2.
+%%%
+%%% The function isolate_nifs/1 transforms the input #b_module{} by
+%%% rewriting all calls to NIFs in the module to calls to external
+%%% functions with the same names.  As this also removes all callers
+%%% to the NIF, all non-exported NIFs are forcibly exported, to avoid
+%%% them being removed as dead code.
+%%%
+%%% As all passes know how handle external calls, this allows for safe
+%%% optimization of the module. That a NIF function can contain BEAM
+%%% code which calls other functions in the module is not a problem,
+%%% at worst it leads to missed optimizations.
+%%%
+%%% When all sub-passes of beam_ssa_opt have been executed
+%%% restore_nifs/2 undoes the module transforms done by
+%%% isolate_nifs/1. To avoid extra book-keeping to keep track of
+%%% rewritten calls for use by restore_nifs/2, the module to which the
+%%% calls are redirected is given a name, '\nnifs', which cannot be
+%%% created by the user.
+%%%
+-define(ISOLATION_MODULE, #b_literal{val='\nnifs'}).
+
+isolate_nifs(#b_module{body=Body0, exports=Exports0}=Module0) ->
+    %% Scan to find NIFs
+    NIFs = foldl(fun(#b_function{}=F, Acc) ->
+                         case is_nif(F) of
+                             true ->
+                                 sets:add_element(get_func_id(F), Acc);
+                             false ->
+                                 Acc
+                         end
+                 end, sets:new([{version,2}]), Body0),
+
+    %% Determine the set of previously not exported NIFs which should
+    %% be exported.
+    ExportsSet = foldl(fun({N,A}, Acc) ->
+                               FA = #b_local{name=#b_literal{val=N},arity=A},
+                               sets:add_element(FA, Acc)
+                       end, sets:new([{version,2}]), Exports0),
+    NIFsToExport = sets:subtract(NIFs, ExportsSet),
+    Exports = Exports0 ++ [{N,A}
+                           || #b_local{name=#b_literal{val=N},arity=A}
+                                  <- sets:to_list(NIFsToExport)],
+
+    %% Replace all calls to the NIFs with a call to an external
+    %% function with the same name, but with a module name which
+    %% cannot be created by the user ('\nnifs').
+    CallReplacer =
+        fun(#b_set{op=call,args=[#b_local{name=N,arity=A}=Callee|Rest]}=I)->
+                case sets:is_element(Callee, NIFs) of
+                    true ->
+                        Args = [#b_remote{mod=?ISOLATION_MODULE,
+                                          name=N,arity=A}|Rest],
+                        I#b_set{args=Args};
+                    false ->
+                        I
+                end;
+           (I) ->
+                I
+        end,
+    #b_module{body=Body} = map_module_instrs(CallReplacer, Module0),
+    NIFsAsExternal = sets:fold(fun(#b_local{name=N,arity=A}, Acc) ->
+                                       R = #b_remote{mod=?ISOLATION_MODULE,
+                                                     name=N,arity=A},
+                                       sets:add_element(R, Acc)
+                               end, sets:new([{version,2}]), NIFs),
+    {Module0#b_module{exports=Exports,body=Body},
+     {NIFsToExport, NIFsAsExternal}}.
+
+map_module_instrs(Fun, #b_module{body=Body}=Module) ->
+    Module#b_module{body=[map_module_instrs_f(Fun, F) || F <- Body]}.
+
+map_module_instrs_f(Fun, #b_function{bs=Bs}=F) ->
+    F#b_function{bs=#{Lbl => map_module_instrs_b(Fun, Blk) || Lbl:=Blk <- Bs}}.
+
+map_module_instrs_b(Fun, #b_blk{is=Is}=Blk) ->
+    Blk#b_blk{is=[Fun(I) || I <- Is]}.
+
+restore_nifs(#b_module{exports=Exports0}=Module0, {NIFsToExport, NIFs}) ->
+    %% Remove the NIFs which where were forcibly exported by
+    %% isolate_nifs/1 from the export list.
+    Exports = [E
+               || E={N,A} <- Exports0,
+                  not sets:is_element(#b_local{name=#b_literal{val=N},
+                                               arity=A}, NIFsToExport)],
+
+    %% Restore all calls that were turned into calls to external
+    %% functions in the '\nnifs' module by converting them to local
+    %% calls.
+    CallRestorer =
+        fun(#b_set{op=call,args=[#b_remote{name=N,arity=A}=Callee|Rest]}=I)->
+                case sets:is_element(Callee, NIFs) of
+                    true ->
+                        I#b_set{args=[#b_local{name=N,arity=A}|Rest]};
+                    false ->
+                        I
+                end;
+           (I) ->
+                I
+        end,
+    #b_module{body=Body} = map_module_instrs(CallRestorer, Module0),
+    Module0#b_module{exports=Exports,body=Body}.
+
+%%%
+%%% Predicate to check if a function is the stub for a nif.
+%%%
+is_nif(#b_function{bs=#{0:=#b_blk{is=[#b_set{op=nif_start}|_]}}}) ->
+    true;
+is_nif(_) ->
+    false.
diff --git a/lib/compiler/test/beam_ssa_check_SUITE_data/nifs.erl b/lib/compiler/test/beam_ssa_check_SUITE_data/nifs.erl
new file mode 100644
index 0000000000..d91b00e11c
--- /dev/null
+++ b/lib/compiler/test/beam_ssa_check_SUITE_data/nifs.erl
@@ -0,0 +1,61 @@
+%% %CopyrightBegin%
+%%
+%% Copyright Ericsson AB 2023. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%
+%% %CopyrightEnd%
+%%
+%% This module tests that beam_ssa_opt:opt/2 correctly handles modules
+%% containing nifs.
+%%
+-module(nifs).
+
+-export([load_nif/0, calling_normal_fun/0, calling_nif/0]).
+
+-nifs([a_nif/0]).
+
+load_nif() ->
+    ok = erlang:load_nif("dummy", 0).
+
+not_a_nif() ->
+    1.
+
+a_nif() ->
+    2.
+
+%% If beam_ssa_opt:isolate_nifs/1 and beam_ssa_opt:restore_nifs/2 fail
+%% to do their jobs, and somehow disable beam_ssa_opt-optimizations,
+%% the result of not_a_nif() + not_a_nif() won't be statically
+%% evaluated.
+calling_normal_fun() ->
+%ssa% () when post_ssa_opt ->
+%ssa% ret(2),
+%ssa% label 1,
+%ssa% ret(_).
+    not_a_nif() + not_a_nif().
+
+%% If beam_ssa_opt:isolate_nifs/1 and beam_ssa_opt:restore_nifs/2 fail
+%% to do their jobs and somehow allow the function marked as a NIF to
+%% be statically evaluated, the addition will have been removed. Also
+%% check that the local calls to the NIFs have been restored to a call
+%% within the module.
+calling_nif() ->
+%ssa% () when post_ssa_opt ->
+%ssa% A = call(fun a_nif/0),
+%ssa% B = call(fun a_nif/0),
+%ssa% Sum = bif:'+'(A, B),
+%ssa% ret(Sum),
+%ssa% label 1,
+%ssa% ret(_).
+    a_nif() + a_nif().
-- 
2.35.3

Places

File 1921-compiler-Allow-ssa_opt-to-run-on-modules-with-NIFs.patch of Package erlang (Revision a95c38533dd94a5c195230b3b146d029)

Places