sysdeps/mach/hurd/profil.c (was: [PATCH] hurd: align -p and -pg behavior on Linux)

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

On Wed, 24 Feb 2016 23:46:36 +0100, I wrote:
> On Sat, 19 Sep 2015 14:00:23 +0200, Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > On Linux, -p and -pg do not make gcc link against libc_p.a, only
> > -profile does (as documented in r11246), and thus people expect -p
> 
> (Yo, 20 years ago...)

Now looking at glibc code of a similar vintage.  ;-)

> In my understanding, the Hurd needs crt0.o for static linking, and crt1.o
> for dynamic linking.  Likewise, for -pg or -p, I would assume that we
> still need gcrt0.o for static linking, and gcrt1.o for dynamic linking.

> I'm now testing the following patch: [...]
> ..., which results in the following spec changes:
> [...]
>  *startfile:
> -%{!shared: %{pg|p|profile:gcrt0.o%s;pie:Scrt1.o%s;static:crt0.o%s;:crt1.o%s}}    crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}
> +%{!shared: %{pg|p|profile:%{static:gcrt0.o%s;:gcrt1.o%s};pie:Scrt1.o%s;static:crt0.o%s;:crt1.o%s}}    crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}

According to the GCC testsuite, that seems alright: a bunch of
UNSUPPORTED -> PASS progressions in gcc.sum.  (But I have not yet done
any manual testing with gprof and such.)

I also see a handful of executions FAILs, the first of which I've now
looked into.  I'll work on getting a glibc build going, to patch this up,
but posting my analysis already, in case anyone has any comments.
(Roland?)

    $ gcc/xgcc -Bgcc/ ../trunk/gcc/testsuite/gcc.dg/20021014-1.c -O2 -p -o ./20021014-1.exe
    $ gdb -q 20021014-1.exe 
    Reading symbols from 20021014-1.exe...done.
    (gdb) r
    Starting program: [...]/20021014-1.exe 
    [New Thread 15527.5]
    [New Thread 15527.6]
    
    Program received signal SIGFPE, Arithmetic exception.
    0x01173155 in fetch_samples () at ../sysdeps/mach/hurd/profil.c:164
    164     ../sysdeps/mach/hurd/profil.c: No such file or directory.
    (gdb) bt
    #0  0x01173155 in fetch_samples () at ../sysdeps/mach/hurd/profil.c:164
    #1  0x01173365 in __profil (sample_buffer=0x0, size=0, offset=0, scale=0) at ../sysdeps/mach/hurd/profil.c:126
    #2  0x0117292d in __moncontrol (mode=0) at gmon.c:93
    #3  0x01172b56 in _mcleanup () at gmon.c:425
    #4  0x0109c617 in __run_exit_handlers (status=0, listp=0x122055c <__exit_funcs>, run_list_atexit=true) at exit.c:82
    #5  0x0109c661 in exit (status=0) at exit.c:104
    #6  0x080484cd in main () at ../trunk/gcc/testsuite/gcc.dg/20021014-1.c:24

That is the "special_profil_failure" in
../sysdeps/mach/hurd/profil.c:fetch_samples.

    (gdb) print special_profil_failure 
    $1 = EMACH_RCV_INVALID_NAME

[glibc]/sysdeps/mach/hurd/bits/errno.h:

    [...]
            /* Errors from <mach/message.h>.  */
    [...]
            EMACH_RCV_INVALID_NAME          = 0x10004002,
    [...]

[gnumach]/include/mach/message.h:

    [...]
    #define MACH_RCV_INVALID_NAME           0x10004002
                    /* Bogus name for receive port/port-set. */
    [...]

We do have a profile thread created:

    (gdb) print profile_thread 
    $2 = 63
    (gdb) info threads
      Id   Target Id         Frame 
      6    Thread 15527.6    profile_waiter () at ../sysdeps/mach/hurd/profil.c:183
      5    Thread 15527.5    0x0105c92c in mach_msg_trap () at /build/glibc-GlHAly/glibc-2.21/build-tree/hurd-i386-libc/mach/mach_msg_trap.S:2
    * 4    Thread 15527.4    0x01173155 in fetch_samples () at ../sysdeps/mach/hurd/profil.c:164
      3    bogus thread id 3 Can't fetch registers from thread bogus thread id 3: No such thread

..., but it's not yet been scheduled for execution:

    (gdb) thread 6
    [Switching to thread 6 (Thread 15527.6)]
    #0  profile_waiter () at ../sysdeps/mach/hurd/profil.c:183
    183     in ../sysdeps/mach/hurd/profil.c
    (gdb) bt
    #0  profile_waiter () at ../sysdeps/mach/hurd/profil.c:183
    Backtrace stopped: Cannot access memory at address 0x2285000
    (gdb) disassemble 
    Dump of assembler code for function profile_waiter:
    => 0x011731b0 <+0>:     push   %ebp
       0x011731b1 <+1>:     push   %edi
       0x011731b2 <+2>:     push   %esi
       0x011731b3 <+3>:     push   %ebx
       0x011731b4 <+4>:     mov    $0x1,%esi
       0x011731b9 <+9>:     call   0x11a06a9 <__x86.get_pc_thunk.bx>
    [...]

That is, the profile thread has not actually began to execute, when the
main thread already tears down the process.  During that, the main thread
talks to profil_reply_port -- but that one is (to be) set up by the
profile thread, which has not yet happened:

    (gdb) print profil_reply_port
    $3 = 0

The scenario is not surprising: [gcc]/gcc/testsuite/gcc.dg/20021014-1.c
is just a few lines of code.

Should we move the initialization of profil_reply_port elsewhere, or be
prepared for profil_reply_port to be MACH_PORT_NULL in
sysdeps/mach/hurd/profil.c:fetch_samples (by returning early?), or not
call fetch_samples from sysdeps/mach/hurd/profil.c:__profil if
profil_reply_port is MACH_PORT_NULL, or make sure that we're always
properly initialized by making sure that the profile thread is always
scheduled to execute right after it's been created?

As far as I can tell, we'd run into the same problem as discussed here,
if profil (sysdeps/mach/hurd/profil.c:__profil) is called to disable
sampling, but it has never been called to enable sampling.

Also, according to the man page, profil
(sysdeps/mach/hurd/profil.c:__profil) should disable profiling if
sample_buffer is NULL, not if scale is zero, as it's currently doing.

As there are accesses to variables shared between different threads,
should these be re-written to use GCC's atomic/sync load/store builtins
with appropriate semantics?

These days, do we still need the threadvar-avoidance magic, the "special
RPC stubs", and the "special_profil_failure"?


GrÃÃe
 Thomas

Attachment: signature.asc
Description: PGP signature