Patchwork Copy x86_64 _mcount.op from _mcount.o

login
register
mail settings
Submitter H.J. Lu
Date March 4, 2016, 1:40 p.m.
Message ID <CAMe9rOr39A0tj5FgpC4Q-3a2EdTgbQorFdNzt6zq+ikZcG6qaA@mail.gmail.com>
Download mbox | patch
Permalink /patch/11193/
State New
Headers show

Comments

H.J. Lu - March 4, 2016, 1:40 p.m.
On Thu, Mar 3, 2016 at 4:19 PM, Roland McGrath <roland@hack.frob.com> wrote:
>> On Thu, Mar 3, 2016 at 4:10 PM, Roland McGrath <roland@hack.frob.com> wrote:
>>
>> > If I'd had a chance to review this change, I'd have said that I don't see
>> > any adequate rationale for this change.
>>
>> It's a pre-condition for using ENTRY/END here:
>> https://sourceware.org/ml/libc-alpha/2016-03/msg00055.html
>
> That needs to be said in the patch posting, and in comments in the makefile.

Like this?
Paul Pluzhnikov - March 4, 2016, 4:02 p.m.
On Fri, Mar 4, 2016 at 5:40 AM, H.J. Lu <hjl.tools@gmail.com> wrote:

> Like this?
>
> --
> H.J.
> ---
> diff --git a/sysdeps/x86_64/Makefile b/sysdeps/x86_64/Makefile
> index 9fcadd8..788e4fc 100644
> --- a/sysdeps/x86_64/Makefile
> +++ b/sysdeps/x86_64/Makefile
> @@ -7,6 +7,9 @@ endif
>
>  ifeq ($(subdir),gmon)
>  sysdep_routines += _mcount
> +# We cannot compile _mcount.S with -pg because that would create
> +# recursive calls when ENTRY is used.  Just copy the normal static
> +# object.
>  sysdep_noprof += _mcount
>  endif

Looks good to me.

Thanks,
H.J. Lu - March 4, 2016, 4:46 p.m.
On Fri, Mar 4, 2016 at 8:02 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> On Fri, Mar 4, 2016 at 5:40 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>> Like this?
>>
>> --
>> H.J.
>> ---
>> diff --git a/sysdeps/x86_64/Makefile b/sysdeps/x86_64/Makefile
>> index 9fcadd8..788e4fc 100644
>> --- a/sysdeps/x86_64/Makefile
>> +++ b/sysdeps/x86_64/Makefile
>> @@ -7,6 +7,9 @@ endif
>>
>>  ifeq ($(subdir),gmon)
>>  sysdep_routines += _mcount
>> +# We cannot compile _mcount.S with -pg because that would create
>> +# recursive calls when ENTRY is used.  Just copy the normal static
>> +# object.
>>  sysdep_noprof += _mcount
>>  endif
>
> Looks good to me.
>
> Thanks,
> --
> Paul Pluzhnikov

Checked in:

commit 97f7112728fa6f5e2b30af4d085d5f4dedb2b89b
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Mar 4 08:44:43 2016 -0800

    Add a comment in sysdeps/x86_64/Makefile

    Mention recursive calls when ENTRY is used in _mcount.S.

      * sysdeps/x86_64/Makefile (sysdep_noprof): Add a comment.
Roland McGrath - March 4, 2016, 6:21 p.m.
> > That needs to be said in the patch posting, and in comments in the makefile.
> 
> Like this?

That comment is a fine addition.  The more important point for you to
understand is the process issue of commits without proper review.
I hope we don't have to have that conversation again in the future.
H.J. Lu - March 4, 2016, 6:26 p.m.
On Fri, Mar 4, 2016 at 10:21 AM, Roland McGrath <roland@hack.frob.com> wrote:
>> > That needs to be said in the patch posting, and in comments in the makefile.
>>
>> Like this?
>
> That comment is a fine addition.  The more important point for you to
> understand is the process issue of commits without proper review.
> I hope we don't have to have that conversation again in the future.

Sure.

BTW, should we do something about timely reviews?

Patch

diff --git a/sysdeps/x86_64/Makefile b/sysdeps/x86_64/Makefile
index 9fcadd8..788e4fc 100644
--- a/sysdeps/x86_64/Makefile
+++ b/sysdeps/x86_64/Makefile
@@ -7,6 +7,9 @@  endif

 ifeq ($(subdir),gmon)
 sysdep_routines += _mcount
+# We cannot compile _mcount.S with -pg because that would create
+# recursive calls when ENTRY is used.  Just copy the normal static
+# object.
 sysdep_noprof += _mcount
 endif