[Bug dynamic-link/29818] New: STAP probe map_failed is no longer generated.

[Bug dynamic-link/29818] New: STAP probe map_failed is no longer generated.

[aburgess at redhat dot com]

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

            Bug ID: 29818
           Summary: STAP probe map_failed is no longer generated.
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: dynamic-link
          Assignee: unassigned at sourceware dot org
          Reporter: aburgess at redhat dot com
  Target Milestone: ---

Created attachment 14472
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14472&action=edit
Remove the map_failed probe.

While working on GDB, I noticed that in recent glibc, the map_failed probe was
no longer being generated within the dynamic linker.

The reason is that the 'map_failed' probe is being optimised out by the
compiler after this commit:

  commit ed3ce71f5c64c5f07cbde0ef03554ea8950d8f2c
  Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
  Date:   Thu Nov 11 09:28:21 2021 -0300

      elf: Move la_activity (LA_ACT_ADD) after _dl_add_to_namespace_list() (BZ
#28062)

The problem is that the map_failed probe is guarded like this:

          if (make_consistent && r != NULL)
            {
              r->r_state = RT_CONSISTENT;
              _dl_debug_state ();
              LIBC_PROBE (map_failed, 2, nsid, r);
            }

After the above commit the make_consistent variable can never be true when this
block of code is reached, as such, the compiler has optimised away this block,
including the map_failed probe within it.

Both the map_start and map_failed probes are in the function
_dl_map_object_from_fd.  Before the above commit it is was possible to reach
the map_start probe, and then later in the function, decide that actually, we
couldn't map the requested file for some reason.

We would then exit _dl_map_object_from_fd, but first we would pass through the
map_failed probe, this would allow observers to know that the mapping,
identified by the previous map_start probe, had failed.

After the above commit, the map_start probe is now only reached at the very end
of the function _dl_map_object_from_fd, when all other error checks have
already been completed.  As such, there is no longer a need for a map_failed
probe.  If an observer sees map_start, then they can know that the mapping will
proceed.

This bug is really just me asking for confirmation of the above understanding. 
From the GDB side, we were never actually using map_start / map_failed, but we
did check for both probes, so the absence of map_failed was causing GDB to not
make use of the probes.  I already plan to remove the map_failed check from GDB
given there are glibc in the wild without that probe.  But it would be nice to
confirm that the probe is now deprecated.

I've attached a patch that removes the map_failed probe, but this is completely
untested.

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

[Bug dynamic-link/29818] STAP probe map_failed is no longer generated.

[aburgess at redhat dot com]

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

Andrew Burgess <aburgess at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |adhemerval.zanella at linaro dot o
                   |                            |rg

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

[Bug dynamic-link/29818] STAP probe map_failed is no longer generated.

[simon.marchi at polymtl dot ca]

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

Simon Marchi <simon.marchi at polymtl dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simon.marchi at polymtl dot ca

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

[Bug dynamic-link/29818] STAP probe map_failed is no longer generated.

[lsix at lancelotsix dot com]

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

Lancelot SIX <lsix at lancelotsix dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |lsix at lancelotsix dot com

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

[Bug dynamic-link/29818] STAP probe map_failed is no longer generated.

[adhemerval.zanella at linaro dot org]

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

Adhemerval Zanella <adhemerval.zanella at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING

--- Comment #1 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
Patch looks ok, thanks. It also needs to remove the map_failed entry from
elf/rtld-debugger-interface.txt, since it is not used anymore. Could you send
the patch on libc-alpha please?

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

[Bug dynamic-link/29818] STAP probe map_failed is no longer generated.

[pedro at palves dot net]

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

Pedro Alves <pedro at palves dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pedro at palves dot net

--- Comment #2 from Pedro Alves <pedro at palves dot net> ---
IWBN if glibc had some kind of test that ensured that all the
meant-to-be-exported probes are really exported, to catch such a problem from
happening again.

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

[Bug dynamic-link/29818] STAP probe map_failed is no longer generated.

[fche at redhat dot com]

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

Frank Ch. Eigler <fche at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fche at redhat dot com

--- Comment #3 from Frank Ch. Eigler <fche at redhat dot com> ---
Interesting!

To Pedro's question, enumerating probes in a compiled libc is a matter of a
perf or readelf or stap command, not a problem.  Such a thing could be a test
case that matches the manual/probes.texi list against what turns out to be
compiled.

Consumers such as gdb should not assume/assert the existence of any particular
probe, if at all possible.

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

[Bug dynamic-link/29818] STAP probe map_failed is no longer generated.

[adhemerval.zanella at linaro dot org]

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

--- Comment #4 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
(In reply to Frank Ch. Eigler from comment #3)
> Interesting!
> 
> To Pedro's question, enumerating probes in a compiled libc is a matter of a
> perf or readelf or stap command, not a problem.  Such a thing could be a
> test case that matches the manual/probes.texi list against what turns out to
> be compiled.
> 
> Consumers such as gdb should not assume/assert the existence of any
> particular probe, if at all possible.

We have a internal script (scripts/glibcelf.py) which has ELF parse support to
check against the elf/elf.h header definition, it should be feasible to extend
it support stapsdt notes and compare against the ones described in
manual/probes.texi (which could be obtained with another script).

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