[Qemu-devel] [PATCH] linux-user: Correct DLINFO_ITEMS

[Qemu-devel] [PATCH] linux-user: Correct DLINFO_ITEMS

[James Hogan]

Commit a07c67dfccb1 (Implement AT_CLKTCK.) back in March 2008 added a
new auxvec entry but didn't increment DLINFO_ITEMS, so it's been out of
sync ever since.

Bump it up to 14 so that it matches the number of NEW_AUX_ENT's that
need to be counted in create_elf_tables().

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Riku Voipio <riku.voipio@iki.fi>
Cc: Paul Brook <paul@codesourcery.com>
---
 linux-user/elfload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 99a2c58..d2380b6 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1076,7 +1076,7 @@ struct exec
 #define TARGET_ELF_PAGESTART(_v) ((_v) & ~(unsigned long)(TARGET_ELF_EXEC_PAGESIZE-1))
 #define TARGET_ELF_PAGEOFFSET(_v) ((_v) & (TARGET_ELF_EXEC_PAGESIZE-1))
 
-#define DLINFO_ITEMS 13
+#define DLINFO_ITEMS 14
 
 static inline void memcpy_fromfs(void * to, const void * from, unsigned long n)
 {
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH] linux-user: Correct DLINFO_ITEMS

[Peter Maydell]

On 25 March 2014 21:47, James Hogan <james.hogan@imgtec.com> wrote:
> Commit a07c67dfccb1 (Implement AT_CLKTCK.) back in March 2008 added a
> new auxvec entry but didn't increment DLINFO_ITEMS, so it's been out of
> sync ever since.
>
> Bump it up to 14 so that it matches the number of NEW_AUX_ENT's that
> need to be counted in create_elf_tables().

This code could clearly use at least an assert that we've not written more
entries than we should, or ideally restructuring somehow so that we don't
have to set DLINFO_ITEMS by hand in the first place...

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
for the immediate fix, though.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] linux-user: Correct DLINFO_ITEMS

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1306 bytes --]

On Tuesday 25 March 2014 22:20:04 Peter Maydell wrote:
> On 25 March 2014 21:47, James Hogan <james.hogan@imgtec.com> wrote:
> > Commit a07c67dfccb1 (Implement AT_CLKTCK.) back in March 2008 added a
> > new auxvec entry but didn't increment DLINFO_ITEMS, so it's been out of
> > sync ever since.
> > 
> > Bump it up to 14 so that it matches the number of NEW_AUX_ENT's that
> > need to be counted in create_elf_tables().
> 
> This code could clearly use at least an assert that we've not written more
> entries than we should, or ideally restructuring somehow so that we don't
> have to set DLINFO_ITEMS by hand in the first place...

Good idea. I've just submitted a patch to add an assert that the allocated 
stack for auxvec/envp/argv matches the amount used.

I've already re-factored a bunch of this code to handle stacks which grow 
upwards (as used for HPPA and Meta arches), which would make it even more 
awkward to avoid DLINFO_ITEMS since it still builds the auxvec, envp and argv 
downwards within a new stack frame (grown upwards), so it also needs to know 
how much stack frame to allocate in advance to be sure it doesn't clobber 
other stuff lower on the stack.

> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> for the immediate fix, though.

Thanks for reviewing,

Cheers
James


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [PATCH] linux-user: Correct DLINFO_ITEMS

[Peter Maydell]

On 25 March 2014 23:40, James Hogan <james.hogan@imgtec.com> wrote:
> I've already re-factored a bunch of this code to handle stacks which grow
> upwards (as used for HPPA and Meta arches)

...we don't support either of those currently, do you plan to submit
something?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] linux-user: Correct DLINFO_ITEMS

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 701 bytes --]

On Tuesday 25 March 2014 23:51:39 Peter Maydell wrote:
> On 25 March 2014 23:40, James Hogan <james.hogan@imgtec.com> wrote:
> > I've already re-factored a bunch of this code to handle stacks which grow
> > upwards (as used for HPPA and Meta arches)
> 
> ...we don't support either of those currently, do you plan to submit
> something?

I'm not sure yet. I have the okay to mainline our Meta QEMU fork[1], but it's 
fallen a little behind (v1.3.1 - and in my experiencing updating an out of 
tree QEMU fork is painful), so I've just been taking another look in my own 
time to see what it would take.

Cheers
James

[1] https://github.com/img-meta/qemu/commit/9355e4b1929c7f05a103dc0e76fe44b54d12b2a7

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [PATCH] linux-user: Correct DLINFO_ITEMS

[Riku Voipio]

Hi,

On Tue, Mar 25, 2014 at 10:20:04PM +0000, Peter Maydell wrote:
> On 25 March 2014 21:47, James Hogan <james.hogan@imgtec.com> wrote:
> > Commit a07c67dfccb1 (Implement AT_CLKTCK.) back in March 2008 added a
> > new auxvec entry but didn't increment DLINFO_ITEMS, so it's been out of
> > sync ever since.
> >
> > Bump it up to 14 so that it matches the number of NEW_AUX_ENT's that
> > need to be counted in create_elf_tables().
> 
> This code could clearly use at least an assert that we've not written more
> entries than we should, or ideally restructuring somehow so that we don't
> have to set DLINFO_ITEMS by hand in the first place...
 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> for the immediate fix, though.

I've applied this and the other paches from you to my tree:

https://git.linaro.org/people/riku.voipio/qemu.git/shortlog/refs/heads/linux-user-for-upstream

I think others are ok to be applied post-2.0 release, but I'll
send this for 2.0

Riku

Riku