Linking: fix up all types before handling functions or objects

Function parameters may require application of type fixes: our linking
ensures that all types have a unique tag, and some parameter's types may
require such tag updates. We used to do this for objects already, but
need to apply it to parameters as well. Also, the order matters: all
types need to be sorted out first before attempting objects or
functions.

Spotted while trying to compile a particular version of Xen 4.11.

Author: tautschnig

regression/ansi-c/linking_conflicts2/test.desc

@@ -3,6 +3,6 @@ main.c
 other.c
 ^EXIT=(64|1)$
 ^SIGNAL=0$
-error: conflicting function declarations 'foo'
+error: duplicate definition of function 'foo'$
 --
 ^warning: ignoring

regression/cbmc/incomplete-structs/test.desc

@@ -1,9 +1,6 @@
 CORE new-smt-backend
 typesmain.c
 types1.c types2.c types3.c
-reason for conflict at \*#this.u: number of members is different \(3/0\)
-reason for conflict at \*#this.u: number of members is different \(0/3\)
-struct U   \(incomplete\)
 warning: pointer parameter types differ between declaration and definition "bar"
 warning: pointer parameter types differ between declaration and definition "foo"
 ^VERIFICATION SUCCESSFUL$

src/linking/linking.cpp

@@ -678,9 +678,18 @@ void linkingt::duplicate_code_symbol(
           n_it!=new_t.parameters().end();
           ++o_it, ++n_it)
       {
-        if(o_it->type() != n_it->type())
+        if(o_it->type() == n_it->type())
+          continue;
+
+        adjust_type_infot info(old_symbol, new_symbol);
+        bool failed = adjust_object_type_rec(o_it->type(), n_it->type(), info);
+        replace = info.set_to_new;
+
+        if(failed)
+        {
           conflicts.push_back(
             std::make_pair(o_it->type(), n_it->type()));
+        }
       }
       if(o_it!=old_t.parameters().end())
       {
@@ -961,22 +970,20 @@ bool linkingt::adjust_object_type_rec(
 
   if(t1.id()==ID_pointer)
   {
-    #if 0
-    bool s=info.set_to_new;
-    if(adjust_object_type_rec(t1.subtype(), t2.subtype(), info))
+    if(info.old_symbol.type.id() == ID_code)
+    {
+      link_warning(
+        info.old_symbol,
+        info.new_symbol,
+        "pointer parameter types differ between declaration and definition");
+    }
+    else
     {
       link_warning(
         info.old_symbol,
         info.new_symbol,
         "conflicting pointer types for variable");
-      info.set_to_new=s;
     }
-    #else
-    link_warning(
-      info.old_symbol,
-      info.new_symbol,
-      "conflicting pointer types for variable");
-    #endif
 
     if(info.old_symbol.is_extern && !info.new_symbol.is_extern)
     {
@@ -1460,7 +1467,7 @@ void linkingt::copy_symbols(
   }
 
   // Move over all the non-colliding ones
-  std::unordered_set<irep_idt> collisions;
+  std::unordered_set<irep_idt> type_collisions, non_type_collisions;
 
   for(const auto &named_symbol : src_symbols)
   {
@@ -1477,21 +1484,27 @@ void linkingt::copy_symbols(
         // new
         main_symbol_table.add(named_symbol.second);
       }
+      else if(named_symbol.second.is_type)
+        type_collisions.insert(named_symbol.first);
       else
-        collisions.insert(named_symbol.first);
+        non_type_collisions.insert(named_symbol.first);
     }
   }
 
   // Now do the collisions
-  for(const irep_idt &collision : collisions)
+  for(const irep_idt &collision : type_collisions)
   {
     symbolt &old_symbol = main_symbol_table.get_writeable_ref(collision);
     symbolt &new_symbol=src_symbols.at(collision);
 
-    if(new_symbol.is_type)
-      duplicate_type_symbol(old_symbol, new_symbol);
-    else
-      duplicate_non_type_symbol(old_symbol, new_symbol);
+    duplicate_type_symbol(old_symbol, new_symbol);
+  }
+  for(const irep_idt &collision : non_type_collisions)
+  {
+    symbolt &old_symbol = main_symbol_table.get_writeable_ref(collision);
+    symbolt &new_symbol = src_symbols.at(collision);
+
+    duplicate_non_type_symbol(old_symbol, new_symbol);
   }
 
   // Apply type updates to initializers