Re: [patch] Ignore non relobj files in gc

[Cary Coutant <ccoutant at gmail dot com>]

> If a relocation points to a dynamic object we can ignore it, since we
> cannot gc an already existing .so file.
>
> The attached patch produces an identical chromium binary, but does so
> a bit faster (see the attached perf logs).

I'd think the one-line patch below accomplishes the same result with
much less disruption:

@@ -340,7 +341,7 @@ gc_process_relocs(
                                                          src_obj));
            }

-          if (gsym->source() != Symbol::FROM_OBJECT)
+          if (dst_obj == NULL || dst_obj->is_dynamic())
             continue;
           if (!is_ordinary)
             continue;

> Using Relobj also helps me in my patch for gcing parts of SHF_MERGE sections.

If that's the case, let's move those changes to a separate patch.
Nevertheless, I think it would be cleaner to change Section_id to use
a Relobj* directly. That has some cascading effects, but doesn't
affect quite as much target-independent code, and we really never need
a Section_id that refers to a non-relocatable object. The attached
patch does that.

Sri, would you mind taking a look to make sure I haven't done anything
in the attached patch that might break --icf? Here's the only place
where we might have created a Section_id with a pointer to a Dynobj,
but I don't think that makes sense:

@@ -324,8 +326,11 @@ gc_process_relocs(
           if (is_icf_tracked)
             {
              Address symvalue = dst_off - addend;
-              if (is_ordinary && gsym->source() == Symbol::FROM_OBJECT)
-               (*secvec).push_back(Section_id(dst_obj, dst_indx));
+              if (is_ordinary
+                 && gsym->source() == Symbol::FROM_OBJECT
+                 && !dst_obj->is_dynamic())
+               (*secvec).push_back(Section_id(static_cast<Relobj*>(dst_obj),
+                                              dst_indx));
              else
                 (*secvec).push_back(Section_id(NULL, 0));
               (*symvec).push_back(gsym);

Perhaps it does make sense here to track a relocation pointing into a
dynamic object, but in this case, the static_cast might still be OK,
since I think at this point, we're only using the object pointer as an
opaque ID for the object.

-cary

Attachment: gold-section-id.patch
Description: Binary data