Merge branch 'siri/cdv/misc-improvements/OTP-14755' into maint

* siri/cdv/misc-improvements/OTP-14755:
  [cdv] Fix test of size limit reached on crash dump
  [cdv] Display warning if heap is incomplete
  [cdv] Check if dump version is newer than cdv can handle
  [cdv] Add comment about ets tables

2 files changed, 154 insertions, 39 deletions

diff --git a/lib/observer/src/crashdump_viewer.erl b/lib/observer/src/crashdump_viewer.erl
index ddf2879a97..40450a2873 100644
--- a/lib/observer/src/crashdump_viewer.erl
+++ b/lib/observer/src/crashdump_viewer.erl
@@ -26,10 +26,25 @@
 %% Tables
 %% ------
 %% cdv_dump_index_table: This table holds all tags read from the
-%% crashdump.  Each tag indicates where the information about a
-%% specific item starts.  The table entry for a tag includes the start
-%% position for this item-information. In a crash dump file, all tags
-%% start with a "=" at the beginning of a line.
+%% crashdump, except the 'binary' tag.  Each tag indicates where the
+%% information about a specific item starts.  The table entry for a
+%% tag includes the start position for this item-information. In a
+%% crash dump file, all tags start with a "=" at the beginning of a
+%% line.
+%%
+%% cdv_binary_index_table: This table holds all 'binary' tags. The hex
+%% address for each binary is converted to its integer value before
+%% storing Address -> Start Position in this table. The hex value of
+%% the address is never used for lookup.
+%%
+%% cdv_reg_proc_table: This table holds mappings between pid and
+%% registered name. This is used for timers and monitors.
+%%
+%% cdv_heap_file_chars: For each 'proc_heap' and 'literals' tag, this
+%% table contains the number of characters to read from the crash dump
+%% file. This is used for giving an indication in percent of the
+%% progress when parsing this data.
+%%
 %%
 %% Process state
 %% -------------
@@ -73,6 +88,9 @@
 -export([init/1, handle_call/3, handle_cast/2, handle_info/2, 
 	 terminate/2, code_change/3]).
 
+%% Test support
+-export([get_dump_versions/0]).
+
 %% Debug support
 -export([debug/1,stop_debug/0]).
 
@@ -87,6 +105,7 @@
 			    % line_head/1 function can return
 -define(not_available,"N/A").
 -define(binary_size_progress_limit,10000).
+-define(max_dump_version,[0,4]).
 
 
 %% All possible tags - use macros in order to avoid misspelling in the code
@@ -294,6 +313,11 @@ port(Id) ->
 expand_binary(Pos) ->
     call({expand_binary,Pos}).
 
+%%%-----------------------------------------------------------------
+%%% For testing only - called from crashdump_viewer_SUITE
+get_dump_versions() ->
+    call(get_dump_versions).
+
 %%====================================================================
 %% Server functions
 %%====================================================================
@@ -455,8 +479,9 @@ handle_call(index_tables,_From,State=#state{file=File}) ->
 handle_call(schedulers,_From,State=#state{file=File}) ->
     Schedulers=schedulers(File),
     TW = truncated_warning([?scheduler]),
-    {reply,{ok,Schedulers,TW},State}.
-
+    {reply,{ok,Schedulers,TW},State};
+handle_call(get_dump_versions,_From,State=#state{dump_vsn=DumpVsn}) ->
+    {reply,{ok,{?max_dump_version,DumpVsn}},State}.
 
 
 %%--------------------------------------------------------------------
@@ -781,10 +806,8 @@ parse_vsn_str(Str,WS) ->
 
 
 %%%-----------------------------------------------------------------
-%%% Traverse crash dump and insert index in table for each heading
-%%%
-%%% Progress is reported during the time and MUST be checked with
-%%% crashdump_viewer:get_progress/0 until it returns {ok,done}.
+%%% Traverse crash dump and insert index in table for each heading.
+%%% Progress is reported during the time.
 do_read_file(File) ->
     erase(?literals),                           %Clear literal cache.
     put(truncated,false),                       %Not truncated (yet).
@@ -800,17 +823,21 @@ do_read_file(File) ->
 		    {Tag,Id,Rest,N1} = tag(Fd,TagAndRest,1),
 		    case Tag of
 			?erl_crash_dump ->
-			    reset_tables(),
-			    insert_index(Tag,Id,N1+1),
-			    put_last_tag(Tag,""),
-                            DumpVsn = [list_to_integer(L) ||
-                                          L<-string:tokens(Id,".")],
-                            AddrAdj = get_bin_addr_adj(DumpVsn),
-                            indexify(Fd,AddrAdj,Rest,N1),
-			    end_progress(),
-			    check_if_truncated(),
-			    close(Fd),
-                            {ok,DumpVsn};
+                            case check_dump_version(Id) of
+                                {ok,DumpVsn} ->
+                                    reset_tables(),
+                                    insert_index(Tag,Id,N1+1),
+                                    put_last_tag(Tag,""),
+                                    AddrAdj = get_bin_addr_adj(DumpVsn),
+                                    indexify(Fd,AddrAdj,Rest,N1),
+                                    end_progress(),
+                                    check_if_truncated(),
+                                    close(Fd),
+                                    {ok,DumpVsn};
+                                Error ->
+                                    close(Fd),
+                                    Error
+                            end;
 			_Other ->
 			    R = io_lib:format(
 				  "~ts is not an Erlang crash dump~n",
@@ -838,6 +865,18 @@ do_read_file(File) ->
 	    {error,R}
     end.
 
+check_dump_version(Vsn) ->
+    DumpVsn = [list_to_integer(L) || L<-string:tokens(Vsn,".")],
+    if DumpVsn > ?max_dump_version ->
+            Info =
+                "This Crashdump Viewer is too old for the given "
+                "Erlang crash dump. Please use a newer version of "
+                "Crashdump Viewer.",
+            {error,Info};
+       true ->
+            {ok,DumpVsn}
+    end.
+
 indexify(Fd,AddrAdj,Bin,N) ->
     case binary:match(Bin,<<"\n=">>) of
 	{Start,Len} ->
@@ -1083,7 +1122,7 @@ get_proc_details(File,Pid,WS,DumpVsn) ->
 	    {{Stack,MsgQ,Dict},TW} =
 		case truncated_warning([{?proc,Pid}]) of
 		    [] ->
-			{expand_memory(Fd,Pid,DumpVsn),[]};
+                        expand_memory(Fd,Pid,DumpVsn);
 		    TW0 ->
 			{{[],[],[]},TW0}
 		end,
@@ -1418,7 +1457,15 @@ expand_memory(Fd,Pid,DumpVsn) ->
 		read_messages(Fd,Pid,BinAddrAdj,Dict),
 		read_dictionary(Fd,Pid,BinAddrAdj,Dict)},
     erase(fd),
-    Expanded.
+    IncompleteWarning =
+        case erase(incomplete_heap) of
+            undefined ->
+                [];
+            true ->
+                ["WARNING: This process has an incomplete heap. "
+                 "Some information might be missing."]
+        end,
+    {Expanded,IncompleteWarning}.
 
 read_literals(Fd) ->
     case lookup_index(?literals,[]) of
@@ -2745,6 +2792,7 @@ deref_ptr(Ptr, Line, BinAddrAdj, D0) ->
 	none ->
 	    case get(fd) of
 		end_of_heap ->
+                    put(incomplete_heap,true),
 		    {['#CDVIncompleteHeap'],Line,D0};
 		Fd ->
 		    case bytes(Fd) of
diff --git a/lib/observer/test/crashdump_viewer_SUITE.erl b/lib/observer/test/crashdump_viewer_SUITE.erl
index 86a60e15f4..29b9e406ae 100644
--- a/lib/observer/test/crashdump_viewer_SUITE.erl
+++ b/lib/observer/test/crashdump_viewer_SUITE.erl
@@ -25,7 +25,7 @@
 %% Test functions
 -export([all/0, suite/0,groups/0,init_per_group/2,end_per_group/2,
 	 start_stop/1,load_file/1,not_found_items/1,
-	 non_existing/1,not_a_crashdump/1,old_crashdump/1]).
+	 non_existing/1,not_a_crashdump/1,old_crashdump/1,new_crashdump/1]).
 -export([init_per_suite/1, end_per_suite/1]).
 -export([init_per_testcase/2, end_per_testcase/2]).
 
@@ -83,6 +83,7 @@ all() ->
      non_existing,
      not_a_crashdump,
      old_crashdump,
+     new_crashdump,
      load_file,
      not_found_items
     ].
@@ -212,6 +213,25 @@ not_a_crashdump(Config) when is_list(Config) ->
 
     ok = crashdump_viewer:stop().
 
+%% Try to load a file with newer version than this crashdump viewer can handle
+new_crashdump(Config) ->
+    Dump = hd(?config(dumps,Config)),
+    ok = start_backend(Dump),
+    {ok,{MaxVsn,CurrentVsn}} = crashdump_viewer:get_dump_versions(),
+    if MaxVsn =/= CurrentVsn ->
+            ct:fail("Current dump version is not equal to cdv's max version");
+       true ->
+            ok
+    end,
+    ok = crashdump_viewer:stop(),
+    NewerVsn = lists:join($.,[integer_to_list(X+1) || X <- MaxVsn]),
+    PrivDir = ?config(priv_dir,Config),
+    NewDump = filename:join(PrivDir,"new_erl_crash.dump"),
+    ok = file:write_file(NewDump,"=erl_crash_dump:"++NewerVsn++"\n"),
+    {error, Reason} = start_backend(NewDump),
+    "This Crashdump Viewer is too old" ++_ = Reason,
+    ok = crashdump_viewer:stop().
+
 %% Load files into the tool and view all pages
 load_file(Config) when is_list(Config) ->
     case ?t:is_debug() of
@@ -328,7 +348,7 @@ browse_file(File) ->
 
     io:format("  info read",[]),
 
-    lookat_all_pids(Procs),
+    lookat_all_pids(Procs,is_truncated(File),incomplete_allowed(File)),
     io:format("  pids ok",[]),
     lookat_all_ports(Ports),
     io:format("  ports ok",[]),
@@ -339,6 +359,21 @@ browse_file(File) ->
 
     Procs. % used as second arg to special/2
 
+is_truncated(File) ->
+    case filename:extension(File) of
+        ".trunc"++_ ->
+            true;
+        _ ->
+            false
+    end.
+
+incomplete_allowed(File) ->
+    %% Incomplete heap is allowed for native libs, since some literals
+    %% are not dumped - and for pre OTP-20 (really pre 20.2) releases,
+    %% since literals were not dumped at all then.
+    Rel = get_rel_from_dump_name(File),
+    Rel < 20 orelse test_server:is_native(lists).
+
 special(File,Procs) ->
     case filename:extension(File) of
 	".full_dist" ->
@@ -528,7 +563,7 @@ special(File,Procs) ->
 	    io:format("  process details ok",[]),
 
 	    #proc{dict=Dict} = ProcDetails,
-            io:format("~p\n", [Dict]),
+            %% io:format("~p\n", [Dict]),
             Maps = crashdump_helper:create_maps(),
             Maps = proplists:get_value(maps,Dict),
             io:format("  maps ok",[]),
@@ -548,14 +583,25 @@ verify_binaries([Bin|T1], [['#CDVBin',Offset,Size,Pos]|T2]) ->
 verify_binaries([], []) ->
     ok.
 
-lookat_all_pids([]) ->
+lookat_all_pids([],_,_) ->
     ok;
-lookat_all_pids([#proc{pid=Pid0}|Procs]) ->
+lookat_all_pids([#proc{pid=Pid0}|Procs],TruncAllowed,IncompAllowed) ->
     Pid = pid_to_list(Pid0),
-    {ok,_ProcDetails=#proc{},_ProcTW} = crashdump_viewer:proc_details(Pid),
-    {ok,_Ets,_EtsTW} = crashdump_viewer:ets_tables(Pid),
-    {ok,_Timers,_TimersTW} = crashdump_viewer:timers(Pid),
-    lookat_all_pids(Procs).
+    {ok,_ProcDetails=#proc{},ProcTW} = crashdump_viewer:proc_details(Pid),
+    {ok,_Ets,EtsTW} = crashdump_viewer:ets_tables(Pid),
+    {ok,_Timers,TimersTW} = crashdump_viewer:timers(Pid),
+    case {ProcTW,EtsTW,TimersTW} of
+        {[],[],[]} ->
+            ok;
+        {["WARNING: This process has an incomplete heap."++_],[],[]}
+          when IncompAllowed ->
+            ok;  % native libs, literals might not be included in dump
+        _ when TruncAllowed ->
+            ok; % truncated dump
+        TWs ->
+            ct:fail({unexpected_warning,TWs})
+    end,
+    lookat_all_pids(Procs,TruncAllowed,IncompAllowed).
 
 lookat_all_ports([]) ->
     ok;
@@ -603,13 +649,7 @@ do_create_dumps(DataDir,Rel) ->
 	current ->
 	    CD3 = dump_with_args(DataDir,Rel,"instr","+Mim true"),
 	    CD4 = dump_with_strange_module_name(DataDir,Rel,"strangemodname"),
-	    Tmp = dump_with_args(DataDir,Rel,"trunc_bytes",""),
-            {ok,#file_info{size=Max}} = file:read_file_info(Tmp),
-            ok = file:delete(Tmp),
-            Bytes = max(15,rand:uniform(Max)),
-            CD5 = dump_with_args(DataDir,Rel,"trunc_bytes",
-                                 "-env ERL_CRASH_DUMP_BYTES " ++
-                                     integer_to_list(Bytes)),
+            CD5 = dump_with_size_limit_reached(DataDir,Rel,"trunc_bytes"),
             CD6 = dump_with_unicode_atoms(DataDir,Rel,"unicode"),
             CD7 = dump_with_maps(DataDir,Rel,"maps"),
             TruncatedDumps = truncate_dump(CD1),
@@ -722,6 +762,28 @@ dump_with_strange_module_name(DataDir,Rel,DumpName) ->
     ?t:stop_node(n1),
     CD.
 
+dump_with_size_limit_reached(DataDir,Rel,DumpName) ->
+    Tmp = dump_with_args(DataDir,Rel,DumpName,""),
+    {ok,#file_info{size=Max}} = file:read_file_info(Tmp),
+    ok = file:delete(Tmp),
+    dump_with_size_limit_reached(DataDir,Rel,DumpName,Max).
+
+dump_with_size_limit_reached(DataDir,Rel,DumpName,Max) ->
+    Bytes = max(15,rand:uniform(Max)),
+    CD = dump_with_args(DataDir,Rel,DumpName,
+                        "-env ERL_CRASH_DUMP_BYTES " ++
+                            integer_to_list(Bytes)),
+    {ok,#file_info{size=Size}} = file:read_file_info(CD),
+    if Size < Bytes ->
+            %% This means that the dump was actually smaller than the
+            %% randomly selected truncation size, so we'll just do it
+            %% again with a smaller numer
+            ok = file:delete(CD),
+            dump_with_size_limit_reached(DataDir,Rel,DumpName,Size-3);
+       true ->
+            CD
+    end.
+
 dump_with_unicode_atoms(DataDir,Rel,DumpName) ->
     Opt = rel_opt(Rel),
     Pz = "-pz \"" ++ filename:dirname(code:which(?MODULE)) ++ "\"",
@@ -794,6 +856,11 @@ dump_prefix(current) ->
 dump_prefix(Rel) ->
     lists:concat(["r",Rel,"_dump."]).
 
+get_rel_from_dump_name(File) ->
+    Name = filename:basename(File),
+    ["r"++Rel|_] = string:split(Name,"_"),
+    list_to_integer(Rel).
+
 compat_rel(current) ->
     "";
 compat_rel(Rel) ->