fixes calls search in reconstructor (BinaryAnalysisPlatform#863)

* updates roots in reconstructor

Actually, this PR just restores previous behaviour,
that was lost due to lack of tests and comments.

So this PR:
1) extends a list of roots with addresses of such CFG nodes,
   that don't have any input edges. The effect is noticeable in
   case of pure code input, since the list of roots is empty
   in such case, and therefore the reconstructor will create
   just an empty symtab.
2) adds comments in code to avoid of loosing such behaviour in
   future.
3) updated documentation for reconstructor in `bap.mli`, since
   it looks like quite outdated

* updated testsuite

* updated comments

lib/bap/bap.mli

@@ -7898,20 +7898,17 @@ module Std : sig
     (** [create f] creates a reconstructor from a given function [f]  *)
     val create : (cfg -> symtab) -> t
 
-    (** [default name roots] builds a reconstructor from given a
+    (** [default name roots] builds a reconstructor from a given
         function, that maps addresses to function names (see
         {!Symbolizer}) and a list of known function starts. The
-        reconstructor will extend the list of function start with
-        destinations of call instructions found in the CFG, and then
-        for each function start build a function using the following
-        definition of a function:
+        reconstructor will extend the list of function starts with
+        destinations of call instructions found in the CFG. Also,
+        the reconstructor treats every node without input edges as
+        a function start. For each function start builds a function
+        using the following definition of a function:
 
          Function is built from the entry block and every block that
-         is reachable from it without using calls, if the block
-         address is greater than the entry block address and less
-         than the address of entry block of the next symbol.
-
-        Note: this is an approximation, that works fine for most cases.  *)
+         is reachable from it without using calls. *)
     val default : (word -> string) -> word list -> t
 
 

lib/bap_disasm/bap_disasm_reconstructor.ml

@@ -19,9 +19,36 @@ type reconstructor = t
 let create f = Reconstructor f
 let run (Reconstructor f) = f
 
+(* [is_start roots cfg blk] returns true if [blk] is a function start.
+
+   A block is considered to be a function start if one of the
+   following is true:
+   - the block address belongs to the provided set of roots;
+   - the block has no incoming edges
+
+   In general, all blocks should be reachable from the set of roots,
+   thus a block without incoming edges should belong to the set of
+   roots, so it might be tempting to say that the second clause is
+   redundant.
+
+   However, our implementation of the recursive descent disassembler
+   can generate blocks that are not reachable from the initial set of
+   roots. This can happen only if the set of roots is empty, which is
+   treated as a special case, that instructs the disassembler to treat
+   the first byte of the provided input as a root.
+
+   Beyond being a questionable design decision, this behavior is
+   actually just an example of a more general problem. The
+   reconstructor and disassemblers could be called with different sets
+   of roots. So, in general, we can't rely on the set of roots passed
+   to the reconstructor for functions start classification.*)
+let is_start roots cfg blk =
+  Set.mem roots (Block.addr blk) ||
+  Cfg.Node.degree ~dir:`In blk cfg = 0
+
 let entries_of_block cfg roots entries blk =
   let entries =
-    if Set.mem roots (Block.addr blk) then Set.add entries blk
+    if is_start roots cfg blk then Set.add entries blk
     else entries in
   let term = Block.terminator blk in
   if Insn.(is call) term then