[Bug dynamic-link/24136] New: _dl_profile_fixup set local init to 1 after initialization but doesn't need to.

carlos at redhat dot com sourceware-bugzilla@sourceware.org
Thu Jan 24 21:08:00 GMT 2019


https://sourceware.org/bugzilla/show_bug.cgi?id=24136

            Bug ID: 24136
           Summary: _dl_profile_fixup set local init to 1 after
                    initialization but doesn't need to.
           Product: glibc
           Version: 2.30
            Status: NEW
          Severity: normal
          Priority: P2
         Component: dynamic-link
          Assignee: unassigned at sourceware dot org
          Reporter: carlos at redhat dot com
  Target Milestone: ---

In elf/dl-runtime.c in _dl_profile_fixup():

373       /* Store the result for later runs.  */
374       if (__glibc_likely (! GLRO(dl_bind_not)))
375         {
376           reloc_result->addr = value;
377           /* Guarantee all previous writes complete before
378              init is updated.  See CONCURRENCY NOTES earlier  */
379           atomic_store_release (&reloc_result->init, 1);
380         }
381       init = 1;

We have this 'init = 1' but never check init later, and this is confusing for
anyone reviewing the code.

I reviewed this change, and in the past we used to use init later on in the
code, but then realized it wasn't needed and removed the use, but forgot to
remove the store.

It could all be simplified to just moving the atomic_load_acquire into the
initial if() case e.g. 

213   unsigned int init = atomic_load_acquire (&reloc_result->init);
214 
215   if (init == 0)

Should become:

 if (atomic_load_acquire (&reloc_result->init) == 0)

-- 
You are receiving this mail because:
You are on the CC list for the bug.


More information about the Glibc-bugs mailing list