| Message ID | 20200821222830.91463-1-carenas@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | linux-user: warn if trying to use qemu-mipsn32[el] with non n32 ELF | expand |
Le 22/08/2020 à 00:28, Carlo Marcelo Arenas Belón a écrit : > While technically compatible will (depending on the CPU) hopefully fail > later with a cryptic error: > > qemu: Unexpected FPU mode > > Provide an earlier hint of what the problem might be by detecting if the > binary might not be using the n32 ABI and print a warning. > > Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> > --- > linux-user/elfload.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/linux-user/elfload.c b/linux-user/elfload.c > index fe9dfe795d..64c3921cd9 100644 > --- a/linux-user/elfload.c > +++ b/linux-user/elfload.c > @@ -2390,6 +2390,13 @@ static void load_elf_image(const char *image_name, int image_fd, > if (!elf_check_ehdr(ehdr)) { > goto exit_errmsg; > } > +#ifdef TARGET_ABI_MIPSN32 > +/* from arch/mips/include/asm/elf.h */ > +#define EF_MIPS_ABI2 0x00000020 This is already defined in include/elf.h > + if (!(ehdr->e_flags & EF_MIPS_ABI2)) { > + fprintf(stderr, "warning: ELF binary missing n32 flag\n"); I think an exit would be more appropriate: errmsg = "warning: ELF binary missing n32 flag"; goto exit_errmsg; > + } > +#endif > > i = ehdr->e_phnum * sizeof(struct elf_phdr); > if (ehdr->e_phoff + i <= BPRM_BUF_SIZE) { > CC'ing mips maintainers (and Maciej as he sent a patch on the N32 topic 2 years ago...) Thanks, Laurent
On Saturday, August 22, 2020, Laurent Vivier <laurent@vivier.eu> wrote: > Le 22/08/2020 à 00:28, Carlo Marcelo Arenas Belón a écrit : > > While technically compatible will (depending on the CPU) hopefully fail > > later with a cryptic error: > > > > qemu: Unexpected FPU mode > > > > Provide an earlier hint of what the problem might be by detecting if the > > binary might not be using the n32 ABI and print a warning. > > > > Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> > > --- > > linux-user/elfload.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/linux-user/elfload.c b/linux-user/elfload.c > > index fe9dfe795d..64c3921cd9 100644 > > --- a/linux-user/elfload.c > > +++ b/linux-user/elfload.c > > @@ -2390,6 +2390,13 @@ static void load_elf_image(const char > *image_name, int image_fd, > > if (!elf_check_ehdr(ehdr)) { > > goto exit_errmsg; > > } > > +#ifdef TARGET_ABI_MIPSN32 > > +/* from arch/mips/include/asm/elf.h */ > > +#define EF_MIPS_ABI2 0x00000020 > > This is already defined in include/elf.h > > > + if (!(ehdr->e_flags & EF_MIPS_ABI2)) { > > + fprintf(stderr, "warning: ELF binary missing n32 flag\n"); > > I think an exit would be more appropriate: > > errmsg = "warning: ELF binary missing n32 flag"; > goto exit_errmsg; > Carlo, Laurent, I agree with both Laurent's remarks above. At the same time, I also support the spirit of Carlo's patch - but please, Carlo, if possible, send the v2 that is going to address Laurent's concerns. Moreover, Carlo, please consider doing more cleanup in this area. For example, is the case "attempt passing n32 executable to non-n32 qemu" handled in a user-friendly manner? What about "passing 64-bit executable to 32-bit qemu"? What about opposite endians? Are all user visible messages clear and appropriate? Could you do a thourough analysis in this area? There are 36 combinations (6x6, given 6 mips supported ABIs). I would prefer a complete solution for all these mips combinations "executable vs qemu" in linux user. Perhaps you can send a series of patches on this topic, this one being, let's say, just the first one in that series. Yours, Aleksandar > > + } > > +#endif > > > > i = ehdr->e_phnum * sizeof(struct elf_phdr); > > if (ehdr->e_phoff + i <= BPRM_BUF_SIZE) { > > > > CC'ing mips maintainers (and Maciej as he sent a patch on the N32 topic > 2 years ago...) > > Thanks, > Laurent > >
The differences of bit width and endianness are already being taken care of by the current code. The differences in ABI for ILP32 are the only ones missing and so the new version[1] (sorry, not a series since it makes sense as a single change anyway) should be enough to fix any inconsistencies. With this change, all linux-user versions will show the same consistent error when asked to load an incorrect (wrong architecture, endianness or bit width) binary Carlo [1] https://patchwork.ozlabs.org/project/qemu-devel/patch/20200823101703.18451-1-carenas@gmail.com/
diff --git a/linux-user/elfload.c b/linux-user/elfload.c index fe9dfe795d..64c3921cd9 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -2390,6 +2390,13 @@ static void load_elf_image(const char *image_name, int image_fd, if (!elf_check_ehdr(ehdr)) { goto exit_errmsg; } +#ifdef TARGET_ABI_MIPSN32 +/* from arch/mips/include/asm/elf.h */ +#define EF_MIPS_ABI2 0x00000020 + if (!(ehdr->e_flags & EF_MIPS_ABI2)) { + fprintf(stderr, "warning: ELF binary missing n32 flag\n"); + } +#endif i = ehdr->e_phnum * sizeof(struct elf_phdr); if (ehdr->e_phoff + i <= BPRM_BUF_SIZE) {
While technically compatible will (depending on the CPU) hopefully fail later with a cryptic error: qemu: Unexpected FPU mode Provide an earlier hint of what the problem might be by detecting if the binary might not be using the n32 ABI and print a warning. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> --- linux-user/elfload.c | 7 +++++++ 1 file changed, 7 insertions(+)