diff mbox

[2/2] x86,vdso: Fix vdso_install

Message ID fe51e9f5c338d33b1a8b20813441538df5e17add.1402503408.git.luto@amacapital.net (mailing list archive)
State New, archived
Headers show

Commit Message

Andy Lutomirski June 11, 2014, 4:20 p.m. UTC
Rather than monkeying with barely-comprehensible static pattern
rules, just use an explicit loop.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/vdso/Makefile | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

Comments

Josh Boyer June 11, 2014, 5:23 p.m. UTC | #1
On Wed, Jun 11, 2014 at 12:20 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> Rather than monkeying with barely-comprehensible static pattern
> rules, just use an explicit loop.
>
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> ---
>  arch/x86/vdso/Makefile | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
> index 9769df0..b1c70cc 100644
> --- a/arch/x86/vdso/Makefile
> +++ b/arch/x86/vdso/Makefile
> @@ -9,11 +9,6 @@ VDSOX32-$(CONFIG_X86_X32_ABI)  := y
>  VDSO32-$(CONFIG_X86_32)                := y
>  VDSO32-$(CONFIG_COMPAT)                := y
>
> -vdso-install-$(VDSO64-y)       += vdso.so
> -vdso-install-$(VDSOX32-y)      += vdsox32.so
> -vdso-install-$(VDSO32-y)       += $(vdso32-images)
> -
> -
>  # files to link into the vdso
>  vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
>
> @@ -176,15 +171,14 @@ VDSO_LDFLAGS = -fPIC -shared $(call cc-ldoption, -Wl$(comma)--hash-style=sysv) \
>  GCOV_PROFILE := n
>
>  #
> -# Install the unstripped copy of vdso*.so listed in $(vdso-install-y).
> +# Install the unstripped copies of vdso*.so listed in $(vdso-install-y).
>  #
> -quiet_cmd_vdso_install = INSTALL $@
> -      cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
> -$(vdso-install-y): %.so: $(obj)/%.so.dbg FORCE
> -       @mkdir -p $(MODLIB)/vdso
> -       $(call cmd,vdso_install)
> +quiet_cmd_vdso_install = INSTALL $(sofile)
> +      cmd_vdso_install = cp $(obj)/$(sofile).dbg $(MODLIB)/vdso/$(sofile)
>
> -PHONY += vdso_install $(vdso-install-y)
> -vdso_install: $(vdso-install-y)
> +PHONY += vdso_install
> +vdso_install: $(vdso_img_sodbg:%=$(obj)/%) FORCE
> +       @mkdir -p $(MODLIB)/vdso
> +       @$(foreach sofile,$(vdso_img_sodbg:%.dbg=%),$(call recipe-cmd,vdso_install);)

So this does fix the invocation of 'make vdso_install' and the
resulting files look to be accurate to me, with the glaring exception
that now we get e.g. vdso64.so on x86_64 as the installed file instead
of vdso.so.  How much that actually matters, I have no idea.
Plausibly fixed with a symlink if we really need to perhaps.

josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
H. Peter Anvin June 11, 2014, 5:27 p.m. UTC | #2
On 06/11/2014 10:23 AM, Josh Boyer wrote:
> 
> So this does fix the invocation of 'make vdso_install' and the
> resulting files look to be accurate to me, with the glaring exception
> that now we get e.g. vdso64.so on x86_64 as the installed file instead
> of vdso.so.  How much that actually matters, I have no idea.
> Plausibly fixed with a symlink if we really need to perhaps.
> 

You have that problem anyway, no?  After all, there are three different
vdso images for 32 bits, and you can run 32-bit apps on 64-bit systems, too.

Is there realistically any way for the debugger to pick up the correct one?

	-hpa


--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josh Boyer June 11, 2014, 5:33 p.m. UTC | #3
On Wed, Jun 11, 2014 at 1:27 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 06/11/2014 10:23 AM, Josh Boyer wrote:
>>
>> So this does fix the invocation of 'make vdso_install' and the
>> resulting files look to be accurate to me, with the glaring exception
>> that now we get e.g. vdso64.so on x86_64 as the installed file instead
>> of vdso.so.  How much that actually matters, I have no idea.
>> Plausibly fixed with a symlink if we really need to perhaps.
>>
>
> You have that problem anyway, no?  After all, there are three different
> vdso images for 32 bits, and you can run 32-bit apps on 64-bit systems, too.

Yeah, true.

> Is there realistically any way for the debugger to pick up the correct one?

Probably not.  I'm planning on pushing out our first 3.16 build with
these two patches with no symlink.  I very much doubt anyone is going
to complain.  It was just something I noticed.

josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Lutomirski June 11, 2014, 5:42 p.m. UTC | #4
On Wed, Jun 11, 2014 at 10:33 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
> On Wed, Jun 11, 2014 at 1:27 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 06/11/2014 10:23 AM, Josh Boyer wrote:
>>>
>>> So this does fix the invocation of 'make vdso_install' and the
>>> resulting files look to be accurate to me, with the glaring exception
>>> that now we get e.g. vdso64.so on x86_64 as the installed file instead
>>> of vdso.so.  How much that actually matters, I have no idea.
>>> Plausibly fixed with a symlink if we really need to perhaps.
>>>
>>
>> You have that problem anyway, no?  After all, there are three different
>> vdso images for 32 bits, and you can run 32-bit apps on 64-bit systems, too.
>
> Yeah, true.
>
>> Is there realistically any way for the debugger to pick up the correct one?
>
> Probably not.

Sure there is: build ids.  See /usr/lib/debug/.build-id.

It would be great if we could teach the various debugging tools
(libdw?  gdb?  I don't know what's responsible for the search path) to
search both /usr/lib/debug/.build-id and /lib/modules/`uname
-r`/build-id or something like that.

>
> I'm planning on pushing out our first 3.16 build with
> these two patches with no symlink.  I very much doubt anyone is going
> to complain.  It was just something I noticed.

Does the Fedora RPM magic debuginfo script notice these files and
symlink them into the .build-id directory?

Of course, we don't seem to be generating build ids right now.  I
thought we were.  I'll see if I can fix it.

--Andy

>
> josh
Josh Boyer June 11, 2014, 5:45 p.m. UTC | #5
On Wed, Jun 11, 2014 at 1:42 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Wed, Jun 11, 2014 at 10:33 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
>> On Wed, Jun 11, 2014 at 1:27 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>>> On 06/11/2014 10:23 AM, Josh Boyer wrote:
>>>>
>>>> So this does fix the invocation of 'make vdso_install' and the
>>>> resulting files look to be accurate to me, with the glaring exception
>>>> that now we get e.g. vdso64.so on x86_64 as the installed file instead
>>>> of vdso.so.  How much that actually matters, I have no idea.
>>>> Plausibly fixed with a symlink if we really need to perhaps.
>>>>
>>>
>>> You have that problem anyway, no?  After all, there are three different
>>> vdso images for 32 bits, and you can run 32-bit apps on 64-bit systems, too.
>>
>> Yeah, true.
>>
>>> Is there realistically any way for the debugger to pick up the correct one?
>>
>> Probably not.
>
> Sure there is: build ids.  See /usr/lib/debug/.build-id.

Oh, duh.

> It would be great if we could teach the various debugging tools
> (libdw?  gdb?  I don't know what's responsible for the search path) to
> search both /usr/lib/debug/.build-id and /lib/modules/`uname
> -r`/build-id or something like that.
>
>>
>> I'm planning on pushing out our first 3.16 build with
>> these two patches with no symlink.  I very much doubt anyone is going
>> to complain.  It was just something I noticed.
>
> Does the Fedora RPM magic debuginfo script notice these files and
> symlink them into the .build-id directory?
>
> Of course, we don't seem to be generating build ids right now.  I
> thought we were.  I'll see if I can fix it.

My builds have it:

[jwboyer@sb ~]$ file /lib/modules/3.16.0-0.rc0.git1.1.fc21.x86_64/vdso/vdso64.so
/lib/modules/3.16.0-0.rc0.git1.1.fc21.x86_64/vdso/vdso64.so: ELF
64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically
linked, BuildID[sha1]=0x85ab0014891b5be97871e5e1183b761b9d2e8d9d,
stripped
[jwboyer@sb ~]$

I forget if we're doing something special to get that or not.

josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Lutomirski June 11, 2014, 6:45 p.m. UTC | #6
On Wed, Jun 11, 2014 at 10:45 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
> On Wed, Jun 11, 2014 at 1:42 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Wed, Jun 11, 2014 at 10:33 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
>>> On Wed, Jun 11, 2014 at 1:27 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>>>> On 06/11/2014 10:23 AM, Josh Boyer wrote:
>>>>>
>>>>> So this does fix the invocation of 'make vdso_install' and the
>>>>> resulting files look to be accurate to me, with the glaring exception
>>>>> that now we get e.g. vdso64.so on x86_64 as the installed file instead
>>>>> of vdso.so.  How much that actually matters, I have no idea.
>>>>> Plausibly fixed with a symlink if we really need to perhaps.
>>>>>
>>>>
>>>> You have that problem anyway, no?  After all, there are three different
>>>> vdso images for 32 bits, and you can run 32-bit apps on 64-bit systems, too.
>>>
>>> Yeah, true.
>>>
>>>> Is there realistically any way for the debugger to pick up the correct one?
>>>
>>> Probably not.
>>
>> Sure there is: build ids.  See /usr/lib/debug/.build-id.
>
> Oh, duh.
>
>> It would be great if we could teach the various debugging tools
>> (libdw?  gdb?  I don't know what's responsible for the search path) to
>> search both /usr/lib/debug/.build-id and /lib/modules/`uname
>> -r`/build-id or something like that.
>>
>>>
>>> I'm planning on pushing out our first 3.16 build with
>>> these two patches with no symlink.  I very much doubt anyone is going
>>> to complain.  It was just something I noticed.
>>
>> Does the Fedora RPM magic debuginfo script notice these files and
>> symlink them into the .build-id directory?
>>
>> Of course, we don't seem to be generating build ids right now.  I
>> thought we were.  I'll see if I can fix it.
>
> My builds have it:
>
> [jwboyer@sb ~]$ file /lib/modules/3.16.0-0.rc0.git1.1.fc21.x86_64/vdso/vdso64.so
> /lib/modules/3.16.0-0.rc0.git1.1.fc21.x86_64/vdso/vdso64.so: ELF
> 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically
> linked, BuildID[sha1]=0x85ab0014891b5be97871e5e1183b761b9d2e8d9d,
> stripped
> [jwboyer@sb ~]$
>
> I forget if we're doing something special to get that or not.

Apparently not.  file can see it, but apparently objdump can't.  go figure.

>
> josh
Sam Ravnborg June 11, 2014, 6:51 p.m. UTC | #7
On Wed, Jun 11, 2014 at 01:23:59PM -0400, Josh Boyer wrote:
> On Wed, Jun 11, 2014 at 12:20 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> > Rather than monkeying with barely-comprehensible static pattern
> > rules, just use an explicit loop.
> >
> > Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> > ---
> >  arch/x86/vdso/Makefile | 20 +++++++-------------
> >  1 file changed, 7 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
> > index 9769df0..b1c70cc 100644
> > --- a/arch/x86/vdso/Makefile
> > +++ b/arch/x86/vdso/Makefile
> > @@ -9,11 +9,6 @@ VDSOX32-$(CONFIG_X86_X32_ABI)  := y
> >  VDSO32-$(CONFIG_X86_32)                := y
> >  VDSO32-$(CONFIG_COMPAT)                := y
> >
> > -vdso-install-$(VDSO64-y)       += vdso.so
> > -vdso-install-$(VDSOX32-y)      += vdsox32.so
> > -vdso-install-$(VDSO32-y)       += $(vdso32-images)
> > -
> > -
> >  # files to link into the vdso
> >  vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
> >
> > @@ -176,15 +171,14 @@ VDSO_LDFLAGS = -fPIC -shared $(call cc-ldoption, -Wl$(comma)--hash-style=sysv) \
> >  GCOV_PROFILE := n
> >
> >  #
> > -# Install the unstripped copy of vdso*.so listed in $(vdso-install-y).
> > +# Install the unstripped copies of vdso*.so listed in $(vdso-install-y).
> >  #
> > -quiet_cmd_vdso_install = INSTALL $@
> > -      cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
> > -$(vdso-install-y): %.so: $(obj)/%.so.dbg FORCE
> > -       @mkdir -p $(MODLIB)/vdso
> > -       $(call cmd,vdso_install)
> > +quiet_cmd_vdso_install = INSTALL $(sofile)
> > +      cmd_vdso_install = cp $(obj)/$(sofile).dbg $(MODLIB)/vdso/$(sofile)
> >
> > -PHONY += vdso_install $(vdso-install-y)
> > -vdso_install: $(vdso-install-y)
> > +PHONY += vdso_install
> > +vdso_install: $(vdso_img_sodbg:%=$(obj)/%) FORCE
> > +       @mkdir -p $(MODLIB)/vdso
> > +       @$(foreach sofile,$(vdso_img_sodbg:%.dbg=%),$(call recipe-cmd,vdso_install);)

Can we please fix this in a way where we do not need to add stuff to core kbuild.
If the original approach was used then make took care of the looping
and the foreach part was not needed.


	Sam
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Lutomirski June 11, 2014, 7:03 p.m. UTC | #8
On Wed, Jun 11, 2014 at 11:51 AM, Sam Ravnborg <sam@ravnborg.org> wrote:
> On Wed, Jun 11, 2014 at 01:23:59PM -0400, Josh Boyer wrote:
>> On Wed, Jun 11, 2014 at 12:20 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> > Rather than monkeying with barely-comprehensible static pattern
>> > rules, just use an explicit loop.
>> >
>> > Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>> > ---
>> >  arch/x86/vdso/Makefile | 20 +++++++-------------
>> >  1 file changed, 7 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
>> > index 9769df0..b1c70cc 100644
>> > --- a/arch/x86/vdso/Makefile
>> > +++ b/arch/x86/vdso/Makefile
>> > @@ -9,11 +9,6 @@ VDSOX32-$(CONFIG_X86_X32_ABI)  := y
>> >  VDSO32-$(CONFIG_X86_32)                := y
>> >  VDSO32-$(CONFIG_COMPAT)                := y
>> >
>> > -vdso-install-$(VDSO64-y)       += vdso.so
>> > -vdso-install-$(VDSOX32-y)      += vdsox32.so
>> > -vdso-install-$(VDSO32-y)       += $(vdso32-images)
>> > -
>> > -
>> >  # files to link into the vdso
>> >  vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
>> >
>> > @@ -176,15 +171,14 @@ VDSO_LDFLAGS = -fPIC -shared $(call cc-ldoption, -Wl$(comma)--hash-style=sysv) \
>> >  GCOV_PROFILE := n
>> >
>> >  #
>> > -# Install the unstripped copy of vdso*.so listed in $(vdso-install-y).
>> > +# Install the unstripped copies of vdso*.so listed in $(vdso-install-y).
>> >  #
>> > -quiet_cmd_vdso_install = INSTALL $@
>> > -      cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
>> > -$(vdso-install-y): %.so: $(obj)/%.so.dbg FORCE
>> > -       @mkdir -p $(MODLIB)/vdso
>> > -       $(call cmd,vdso_install)
>> > +quiet_cmd_vdso_install = INSTALL $(sofile)
>> > +      cmd_vdso_install = cp $(obj)/$(sofile).dbg $(MODLIB)/vdso/$(sofile)
>> >
>> > -PHONY += vdso_install $(vdso-install-y)
>> > -vdso_install: $(vdso-install-y)
>> > +PHONY += vdso_install
>> > +vdso_install: $(vdso_img_sodbg:%=$(obj)/%) FORCE
>> > +       @mkdir -p $(MODLIB)/vdso
>> > +       @$(foreach sofile,$(vdso_img_sodbg:%.dbg=%),$(call recipe-cmd,vdso_install);)
>
> Can we please fix this in a way where we do not need to add stuff to core kbuild.
> If the original approach was used then make took care of the looping
> and the foreach part was not needed.

Would expanding out the cmd macro here be okay?  Or is that too
dependent on kbuild internals?

I can fiddle with static patterns again (ugh).

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
index 9769df0..b1c70cc 100644
--- a/arch/x86/vdso/Makefile
+++ b/arch/x86/vdso/Makefile
@@ -9,11 +9,6 @@  VDSOX32-$(CONFIG_X86_X32_ABI)	:= y
 VDSO32-$(CONFIG_X86_32)		:= y
 VDSO32-$(CONFIG_COMPAT)		:= y
 
-vdso-install-$(VDSO64-y)	+= vdso.so
-vdso-install-$(VDSOX32-y)	+= vdsox32.so
-vdso-install-$(VDSO32-y)	+= $(vdso32-images)
-
-
 # files to link into the vdso
 vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
 
@@ -176,15 +171,14 @@  VDSO_LDFLAGS = -fPIC -shared $(call cc-ldoption, -Wl$(comma)--hash-style=sysv) \
 GCOV_PROFILE := n
 
 #
-# Install the unstripped copy of vdso*.so listed in $(vdso-install-y).
+# Install the unstripped copies of vdso*.so listed in $(vdso-install-y).
 #
-quiet_cmd_vdso_install = INSTALL $@
-      cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
-$(vdso-install-y): %.so: $(obj)/%.so.dbg FORCE
-	@mkdir -p $(MODLIB)/vdso
-	$(call cmd,vdso_install)
+quiet_cmd_vdso_install = INSTALL $(sofile)
+      cmd_vdso_install = cp $(obj)/$(sofile).dbg $(MODLIB)/vdso/$(sofile)
 
-PHONY += vdso_install $(vdso-install-y)
-vdso_install: $(vdso-install-y)
+PHONY += vdso_install
+vdso_install: $(vdso_img_sodbg:%=$(obj)/%) FORCE
+	@mkdir -p $(MODLIB)/vdso
+	@$(foreach sofile,$(vdso_img_sodbg:%.dbg=%),$(call recipe-cmd,vdso_install);)
 
 clean-files := vdso32-syscall* vdso32-sysenter* vdso32-int80*