[PATCH v2] elf: Fix fences in _dl_find_object_update (bug 28745)

[Szabolcs Nagy]

The 01/07/2022 13:01, Florian Weimer wrote:
> As explained in Hans Boehm, Can Seqlocks Get Along with Programming
> Language Memory Models?, an acquire fence is needed in
> _dlfo_read_success.  The lack of a fence resulted in an observable
> bug on powerpc64le compile-time load reordering.
> 
> The fence in _dlfo_mappings_begin_update has been reordered, turning
> the fence/store sequence into a release MO store equivalent.
> 
> Relaxed MO loads are used on the reader side, and relaxed MO stores
> on the writer side for the shared data, to avoid formal data races.
> This is just to be conservative; it should not actually be necessary
> given how the data is used.
> 
> This commit also fixes the test run time.  The intent was to run it
> for 3 seconds, but 0.3 seconds was enough to uncover the bug very
> occasionally (while 3 seconds did not reliably show the bug on every
> test run).
> 
> ---
> v2: Fix commit message and formatting glitch.
> 

looks good apart from one typo below, but i think the
synchronization can be simplified.

Reviewed-by: Szabolcs Nagy <szabolcs.nagy@arm.com>

> @@ -293,6 +297,19 @@ _dlfo_mappings_end_update_no_switch (void)
>  static inline bool
>  _dlfo_read_success (uint64_t start_version)
>  {
> +  /* See Hans Boehm, Can Seqlocks Get Along with Programming Language
> +     Memory Models?, Section 4.  This is necessary so that loads in
> +     the TM region are not ordered passed the version check below.  */

ordered past

> +  atomic_thread_fence_acquire ();
> +
> +  /* Synchronizes with stores in _dlfo_mappings_begin_update,
> +     _dlfo_mappings_end_update, _dlfo_mappings_end_update_no_switch.
> +     It is important that all stores from the last update have been
> +     visible, and stores from the next updates are not.
> +
> +     Unlike with seqlocks, there is no check for odd versions here
> +     because we have read the unmodified copy (confirmed to be
> +     unmodified by the unchanged version).  */
>    return _dlfo_read_start_version () == start_version;
>  }