This is the mail archive of the mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: ping: [patch 2/6] PIE: Attach binary even after re-prelinked underneath

> gdb/
> 2010-03-29  Jan Kratochvil  <>
> 	* auxv.c (ld_so_xfer_auxv): Do not error on failed read of data_address.
> gdb/testsuite/
> 2010-03-29  Jan Kratochvil  <>
> 	* gdb.base/break-interp.exp (test_attach): Keep $interp changed.  Move
> 	its restore after the <$relink_args != ""> loop.  new comment.

The code changes themselves look reasonable, but as discussed on IRC,
I think the comments could have more info.  If you want, we can commit
the code, and update the comments as a followup patch. I think we can
also converge on the comments within a day or two, so your choice.

> +  /* While it is an error I am not aware how to solve attaching to PIE under
> +     valgrind --db-attach=yes different way when on-disk file has prelink
> +     change in the meantime.  Currently GDB will drop back to procfs_xfer_auxv
> +     (so that at least non-valgrind PIE attachments with prelink change of
> + work).  To make it working even with valgrind is PR 11440 requiring
> +     a valgrind extension.  */

In this case, I think we need to split the explanation that provides
the situation when the memory read error could happen; what we are doing
in this case (returning -1 to fallback on another xfer method) and its
consequences in terms of correctness; and the problem that is left to
solve. Something like this, perhaps?

    /* The location of the _dl_auxv symbol may no longer be correct if was prelinked after the program was started, which can cause
       the memory read at its address to fail.  When that happens, return
       -1 to fallback on another mechanism for retrieving the AUXV.

       While this allows things to work in most cases, this still does not
       work in the case where we are attaching the debugger to a PIE
       running under valgrind and --db-attach=yes (again, when the on-disk file was changed due to prelink after the process was started).
       However, we have not found a solution to that problem yet, and we
       think that we might need a valgrind extension to make it work.
       This is PR 11440.  */

>  	foreach relink {YES NO} {
> -	    if {[prelink$relink $relink_args [file tail $exec]]
> -	        && [copy $interp_saved $interp]} {
> +	    # It would be more correct to also [copy $interp_saved $interp]
> +	    # here to really test just different prelink of $exec.
> +	    # But we would need a separate test for different prelink of
> +	    # where a bug occured.  It is now all merged into this single test.
> +	    if [prelink$relink $relink_args [file tail $exec]] {
>  		# /proc/PID/exe cannot be loaded as it is "EXECNAME (deleted)".
>  		test_attach_gdb $exec $pid $displacement "attach-relink$relink"

As discussed on IRC, let's expand the comment to provide the explanation
that your provided there.

Basically: * what we used to do (the copy) and why (restore to
             prevent an prelink)
           * why we are no longer doing it
             (making sure that we test GDB with being prelinked too)

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]