diff options
Diffstat (limited to 'results/classifier/zero-shot/108/other/1880225')
| -rw-r--r-- | results/classifier/zero-shot/108/other/1880225 | 1122 |
1 files changed, 1122 insertions, 0 deletions
diff --git a/results/classifier/zero-shot/108/other/1880225 b/results/classifier/zero-shot/108/other/1880225 new file mode 100644 index 000000000..6aa6b6a11 --- /dev/null +++ b/results/classifier/zero-shot/108/other/1880225 @@ -0,0 +1,1122 @@ +other: 0.953 +debug: 0.944 +graphic: 0.940 +semantic: 0.936 +permissions: 0.927 +performance: 0.925 +device: 0.924 +PID: 0.921 +vnc: 0.915 +socket: 0.909 +network: 0.899 +files: 0.889 +boot: 0.857 +KVM: 0.841 + +Emulation of some arm programs fail with "Assertion `have_guest_base' failed." + +This issue is observer with QEMU ToT, checked out around May 15th (but I believe it is present in current master too), and wasn't present in QEMU v5.0.0. + +I am using 32-bit Intel(R) Pentium(R) M processor 1.73GHz host. + +Arm cross-compiler is a standard cross-compiler that comes with Debian-based distributions, and gcc version is: + +$ arm-linux-gnueabi-gcc --version +arm-linux-gnueabi-gcc (Debian 8.3.0-2) 8.3.0 + +Compile this program with cross compiler: + +$ arm-linux-gnueabi-gcc -O2 -static toupper_string.c -o toupper_string-arm + +Emulation with QEMU v5.0.0 is correct, and gives expected output: + +$ ~/Build/qemu-5.0.0/build-gcc/arm-linux-user/qemu-arm ./toupper_string-arm +CONTROL RESULT: (toupper_string) + nwlrbbmqbhcdarz owkkyhiddqscdxr jmowfrxsjybldbe fsarcbynecdyggx xpklorellnmpapq + NWLRBBMQBHCDARZ OWKKYHIDDQSCDXR JMOWFRXSJYBLDBE FSARCBYNECDYGGX XPKLORELLNMPAPQ + +While, in case of QEMU master it fails: + +$ ~/Build/qemu-master/build-gcc/arm-linux-user/qemu-arm ./toupper_string-arm +qemu-arm: /home/rtrk/Build/qemu-master/linux-user/elfload.c:2294: probe_guest_base: Assertion `have_guest_base' failed. +Aborted + +There are many other programs that exibit the same behavior. The failure is arm-sprecific. + + +----------------------------------------------------- + +source code: (let's call this file toupper_string.c) (similar file is also in attachment) + + +#include <stdlib.h> +#include <string.h> +#include <stdio.h> +#include <unistd.h> + + +#define MAX_STRING_LENGHT 15 +#define NUMBER_OF_RANDOM_STRINGS 100 +#define DEFAULT_NUMBER_OF_REPETITIONS 30000 +#define MAX_NUMBER_OF_REPETITIONS 1000000000 +#define NUMBER_OF_CONTROL_PRINT_ITEMS 5 + +/* Structure for keeping an array of strings */ +struct StringStruct { + char chars[MAX_STRING_LENGHT + 1]; +}; + +/** + * Sets characters of the given string to random small letters a-z. + * @param s String to get random characters. + * @len Length of the input string. + */ +static void gen_random_string(char *chars, const int len) +{ + static const char letters[] = "abcdefghijklmnopqrstuvwxyz"; + + for (size_t i = 0; i < len; i++) { + chars[i] = letters[rand() % (sizeof(letters) - 1)]; + } + chars[len] = 0; +} + +void main (int argc, char* argv[]) +{ + struct StringStruct random_strings[NUMBER_OF_RANDOM_STRINGS]; + struct StringStruct strings_to_be_uppercased[NUMBER_OF_RANDOM_STRINGS]; + int32_t number_of_repetitions = DEFAULT_NUMBER_OF_REPETITIONS; + int32_t option; + + /* Parse command line options */ + while ((option = getopt(argc, argv, "n:")) != -1) { + if (option == 'n') { + int32_t user_number_of_repetitions = atoi(optarg); + /* Check if the value is a negative number */ + if (user_number_of_repetitions < 1) { + fprintf(stderr, "Error ... Value for option '-n' cannot be a " + "negative number.\n"); + exit(EXIT_FAILURE); + } + /* Check if the value is a string or zero */ + if (user_number_of_repetitions == 0) { + fprintf(stderr, "Error ... Invalid value for option '-n'.\n"); + exit(EXIT_FAILURE); + } + /* Check if the value is too large */ + if (user_number_of_repetitions > MAX_NUMBER_OF_REPETITIONS) { + fprintf(stderr, "Error ... Value for option '-n' cannot be " + "more than %d.\n", MAX_NUMBER_OF_REPETITIONS); + exit(EXIT_FAILURE); + } + number_of_repetitions = user_number_of_repetitions; + } else { + exit(EXIT_FAILURE); + } + } + + /* Create an array of strings with random content */ + srand(1); + for (size_t i = 0; i < NUMBER_OF_RANDOM_STRINGS; i++) { + gen_random_string(random_strings[i].chars, MAX_STRING_LENGHT); + } + + /* Perform uppercasing of a set of random strings multiple times */ + for (size_t j = 0; j < number_of_repetitions; j++) { + /* Copy initial set of random strings to the set to be uppercased */ + memcpy(strings_to_be_uppercased, random_strings, + NUMBER_OF_RANDOM_STRINGS * (MAX_STRING_LENGHT + 1)); + /* Do actual changing case to uppercase */ + for (size_t i = 0; i < NUMBER_OF_RANDOM_STRINGS; i++) { + int k = 0; + + while (strings_to_be_uppercased[i].chars[k]) { + char ch = strings_to_be_uppercased[i].chars[k] - 32; + memcpy((void *)strings_to_be_uppercased[i].chars + k, + &ch, 1); + k++; + } + } + } + + /* Control printing */ + printf("CONTROL RESULT: (toupper_string)\n"); + for (size_t i = 0; i < NUMBER_OF_CONTROL_PRINT_ITEMS; i++) { + printf(" %s", random_strings[i].chars); + } + printf("\n"); + for (size_t i = 0; i < NUMBER_OF_CONTROL_PRINT_ITEMS; i++) { + printf(" %s", strings_to_be_uppercased[i].chars); + } + printf("\n"); +} + + + + +Aleksandar Markovic <email address hidden> writes: + +> Public bug reported: +> +> This issue is observer with QEMU ToT, checked out around May 15th (but I +> believe it is present in current master too), and wasn't present in QEMU +> v5.0.0. +> +> I am using 32-bit Intel(R) Pentium(R) M processor 1.73GHz host. +> +> Arm cross-compiler is a standard cross-compiler that comes with Debian- +> based distributions, and gcc version is: +> +> $ arm-linux-gnueabi-gcc --version +> arm-linux-gnueabi-gcc (Debian 8.3.0-2) 8.3.0 +> +> Compile this program with cross compiler: +> +> $ arm-linux-gnueabi-gcc -O2 -static toupper_string.c -o toupper_string- +> arm +> +> Emulation with QEMU v5.0.0 is correct, and gives expected output: +> +> $ ~/Build/qemu-5.0.0/build-gcc/arm-linux-user/qemu-arm ./toupper_string-arm +> CONTROL RESULT: (toupper_string) +> nwlrbbmqbhcdarz owkkyhiddqscdxr jmowfrxsjybldbe fsarcbynecdyggx xpklorellnmpapq +> NWLRBBMQBHCDARZ OWKKYHIDDQSCDXR JMOWFRXSJYBLDBE FSARCBYNECDYGGX XPKLORELLNMPAPQ +> +> While, in case of QEMU master it fails: +> +> $ ~/Build/qemu-master/build-gcc/arm-linux-user/qemu-arm ./toupper_string-arm +> qemu-arm: /home/rtrk/Build/qemu-master/linux-user/elfload.c:2294: probe_guest_base: Assertion `have_guest_base' failed. +> Aborted +<snip> + +Works for me in our TCG tests on master: + +20:15:43 [alex@zen:~/l/q/b/user.static] review/aarch64-vms-v7|… + ./arm-linux-user/qemu-arm ./tests/tcg/arm-linux-user/toupper +CONTROL RESULT: (toupper_string) + nwlrbbmqbhcdarz owkkyhiddqscdxr jmowfrxsjybldbe fsarcbynecdyggx xpklorellnmpapq + NWLRBBMQBHCDARZ OWKKYHIDDQSCDXR JMOWFRXSJYBLDBE FSARCBYNECDYGGX XPKLORELLNMPAPQ + +I have submitted a fix to the list that affected programs that couldn't +see /proc/self/maps but I guess that isn't the case here. + +-- +Alex Bennée + + +Using bisection, it can be deduced that this behavior appears to be caused by this commit: + + +commit ee94743034bfb443cf246eda4971bdc15d8ee066 (HEAD) +Author: Alex Bennée <email address hidden> +Date: Wed May 13 18:51:28 2020 +0100 + + linux-user: completely re-write init_guest_space + + First we ensure all guest space initialisation logic comes through + probe_guest_base once we understand the nature of the binary we are + loading. The convoluted init_guest_space routine is removed and + replaced with a number of pgb_* helpers which are called depending on + what requirements we have when loading the binary. + + We first try to do what is requested by the host. Failing that we try + and satisfy the guest requested base address. If all those options + fail we fall back to finding a space in the memory map using our + recently written read_self_maps() helper. + + There are some additional complications we try and take into account + when looking for holes in the address space. We try not to go directly + after the system brk() space so there is space for a little growth. We + also don't want to have to use negative offsets which would result in + slightly less efficient code on x86 when it's unable to use the + segment offset register. + + Less mind-binding gotos and hopefully clearer logic throughout. + + Signed-off-by: Alex Bennée <email address hidden> + Acked-by: Laurent Vivier <email address hidden> + + Message-Id: <email address hidden> + +I just want to stress once again that the test was performed on a 32-bit Intel host. + +It appear that there is no problem on Intel 64-bit hosts. + +Perhaps the problem is manifested on all 32-bit hosts. I currently don't have access to any other 320bit host due to remote work. + +The arm is the only target were I noticed this happens. I checked hppa, mips, mipsel, m68k, ppc, and sh4, they ae all fine, with the same program/example, on the same 32-bit Intel host. I did not checked other target except those I just mentioned. + + +Aleksandar Markovic <email address hidden> writes: + +> I just want to stress once again that the test was performed on a 32-bit +> Intel host. + +Ahh - OK that makes sense. I'll see if I can replicate. + + +-- +Alex Bennée + + +The problem may be in int_guest_commpage() - it returns false. + +From gdb debugging session: + +(gdb) p addr +$1 = (void *) 0xb7ffd000 +(gdb) p want +$2 = (void *) 0xffff0000 +(gdb) n +398 if (addr != want) { +(gdb) p qemu_host_page_size +$3 = 4096 +(gdb) l +393 +394 if (addr == MAP_FAILED) { +395 perror("Allocating guest commpage"); +396 exit(EXIT_FAILURE); +397 } +398 if (addr != want) { +399 return false; +400 } +401 +402 /* Set kernel helper versions; rest of page is 0. */ +(gdb) + + +Aleksandar Markovic <email address hidden> writes: + +> The problem may be in int_guest_commpage() - it returns false. +> +>>From gdb debugging session: +> +> (gdb) p addr +> $1 = (void *) 0xb7ffd000 +> (gdb) p want +> $2 = (void *) 0xffff0000 +> (gdb) n +> 398 if (addr != want) { +> (gdb) p qemu_host_page_size +> $3 = 4096 +> (gdb) l +> 393 +> 394 if (addr == MAP_FAILED) { +> 395 perror("Allocating guest commpage"); +> 396 exit(EXIT_FAILURE); +> 397 } +> 398 if (addr != want) { +> 399 return false; +> 400 } +> 401 +> 402 /* Set kernel helper versions; rest of page is 0. */ +> (gdb) + +I'm not totally convinced the calculation that we do to work out the +extended size of the guest space in 32 bit: + + 11:10 alex@debian-buster-i386/i686 [arm/bugs/add-mmap-fallback@github] >./arm-linux-user/qemu-arm tests/tcg/arm-linux-user/sha1 + pgb_static: loaddr: 10000 + pgb_static: loaddr: ffff0000 + pgb_find_hole: ffff0000:10809a8 (1000) + pgb_find_hole: 0:10809a8 + init_guest_commpage: 0xffff0000 -> 0xb7f48000 (4096) + qemu-arm: /home/alex/lsrc/qemu.git/linux-user/elfload.c:2350: probe_guest_base: Assertion `have_guest_base' failed. + Aborted (core dumped) + +Or in fact why we don't do a MAP_FIXED ass we should have ensured we +have enough space allocated for the guest. Richard any ideas? + +-- +Alex Bennée + + +We rely on the pointer to wrap when accessing the high address of the +COMMPAGE so it lands somewhere reasonable. However on 32 bit hosts we +cannot afford just to map the entire 4gb address range. The old mmap +trial and error code handled this by just checking we could map both +the guest_base and the computed COMMPAGE address. + +We can't just manipulate loadaddr to get what we want so we introduce +an offset which pgb_find_hole can apply when looking for a gap for +guest_base that ensures there is space left to map the COMMPAGE +afterwards. + +This is arguably a little inefficient for the one 32 bit +value (kuser_helper_version) we need to keep there given all the +actual code entries are picked up during the translation phase. + +Fixes: ee94743034b +Bug: https://bugs.launchpad.net/qemu/+bug/1880225 +Cc: Bug 1880225 <email address hidden> +Signed-off-by: Alex Bennée <email address hidden> +Cc: Richard Henderson <email address hidden> +Cc: Peter Maydell <email address hidden> +--- + linux-user/elfload.c | 18 ++++++++++-------- + 1 file changed, 10 insertions(+), 8 deletions(-) + +diff --git a/linux-user/elfload.c b/linux-user/elfload.c +index d6027867a1a..31defce95b5 100644 +--- a/linux-user/elfload.c ++++ b/linux-user/elfload.c +@@ -2145,7 +2145,7 @@ static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, lon + + /* Return value for guest_base, or -1 if no hole found. */ + static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, +- long align) ++ long align, uintptr_t offset) + { + GSList *maps, *iter; + uintptr_t this_start, this_end, next_start, brk; +@@ -2171,7 +2171,7 @@ static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, + + this_end = ((MapInfo *)iter->data)->start; + next_start = ((MapInfo *)iter->data)->end; +- align_start = ROUND_UP(this_start, align); ++ align_start = ROUND_UP(this_start + offset, align); + + /* Skip holes that are too small. */ + if (align_start >= this_end) { +@@ -2221,6 +2221,7 @@ static void pgb_static(const char *image_name, abi_ulong orig_loaddr, + { + uintptr_t loaddr = orig_loaddr; + uintptr_t hiaddr = orig_hiaddr; ++ uintptr_t offset = 0; + uintptr_t addr; + + if (hiaddr != orig_hiaddr) { +@@ -2234,18 +2235,19 @@ static void pgb_static(const char *image_name, abi_ulong orig_loaddr, + if (ARM_COMMPAGE) { + /* + * Extend the allocation to include the commpage. +- * For a 64-bit host, this is just 4GiB; for a 32-bit host, +- * the address arithmetic will wrap around, but the difference +- * will produce the correct allocation size. ++ * For a 64-bit host, this is just 4GiB; for a 32-bit host we ++ * need to ensure there is space bellow the guest_base so we ++ * can map the commpage in the place needed when the address ++ * arithmetic wraps around. + */ + if (sizeof(uintptr_t) == 8 || loaddr >= 0x80000000u) { + hiaddr = (uintptr_t)4 << 30; + } else { +- loaddr = ARM_COMMPAGE & -align; ++ offset = (128 * KiB); + } + } + +- addr = pgb_find_hole(loaddr, hiaddr - loaddr, align); ++ addr = pgb_find_hole(loaddr, hiaddr - loaddr, align, offset); + if (addr == -1) { + /* + * If ARM_COMMPAGE, there *might* be a non-consecutive allocation +@@ -2280,7 +2282,7 @@ static void pgb_dynamic(const char *image_name, long align) + * just above that, and maximises the positive guest addresses. + */ + commpage = ARM_COMMPAGE & -align; +- addr = pgb_find_hole(commpage, -commpage, align); ++ addr = pgb_find_hole(commpage, -commpage, align, 0); + assert(addr != -1); + guest_base = addr; + } +-- +2.20.1 + + + +сре, 27. мај 2020. у 12:07 Alex Bennée <email address hidden> је написао/ла: +> +> We rely on the pointer to wrap when accessing the high address of the +> COMMPAGE so it lands somewhere reasonable. However on 32 bit hosts we +> cannot afford just to map the entire 4gb address range. The old mmap +> trial and error code handled this by just checking we could map both +> the guest_base and the computed COMMPAGE address. +> +> We can't just manipulate loadaddr to get what we want so we introduce +> an offset which pgb_find_hole can apply when looking for a gap for +> guest_base that ensures there is space left to map the COMMPAGE +> afterwards. +> +> This is arguably a little inefficient for the one 32 bit +> value (kuser_helper_version) we need to keep there given all the +> actual code entries are picked up during the translation phase. +> +> Fixes: ee94743034b +> Bug: https://bugs.launchpad.net/qemu/+bug/1880225 + +For the scenario in this bug report, for today's master, before and after +applying this patch: + +BEFORE: + +$ ~/Build/qemu-master/build-gcc/arm-linux-user/qemu-arm ./toupper_string-arm +qemu-arm: /home/rtrk/Build/qemu-master/linux-user/elfload.c:2294: +probe_guest_base: Assertion `have_guest_base' failed. +Aborted + +AFTER: + +$ ~/Build/qemu-master/build-gcc/arm-linux-user/qemu-arm ./toupper_string-arm +CONTROL RESULT: (toupper_string) + nwlrbbmqbhcdarz owkkyhiddqscdxr jmowfrxsjybldbe fsarcbynecdyggx xpklorellnmpapq + NWLRBBMQBHCDARZ OWKKYHIDDQSCDXR JMOWFRXSJYBLDBE FSARCBYNECDYGGX XPKLORELLNMPAPQ + +So, it works in my test bed. + +Tested-by: Aleksandar Markovic <email address hidden> + +> Cc: Bug 1880225 <email address hidden> +> Signed-off-by: Alex Bennée <email address hidden> +> Cc: Richard Henderson <email address hidden> +> Cc: Peter Maydell <email address hidden> +> --- +> linux-user/elfload.c | 18 ++++++++++-------- +> 1 file changed, 10 insertions(+), 8 deletions(-) +> +> diff --git a/linux-user/elfload.c b/linux-user/elfload.c +> index d6027867a1a..31defce95b5 100644 +> --- a/linux-user/elfload.c +> +++ b/linux-user/elfload.c +> @@ -2145,7 +2145,7 @@ static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, lon +> +> /* Return value for guest_base, or -1 if no hole found. */ +> static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, +> - long align) +> + long align, uintptr_t offset) +> { +> GSList *maps, *iter; +> uintptr_t this_start, this_end, next_start, brk; +> @@ -2171,7 +2171,7 @@ static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, +> +> this_end = ((MapInfo *)iter->data)->start; +> next_start = ((MapInfo *)iter->data)->end; +> - align_start = ROUND_UP(this_start, align); +> + align_start = ROUND_UP(this_start + offset, align); +> +> /* Skip holes that are too small. */ +> if (align_start >= this_end) { +> @@ -2221,6 +2221,7 @@ static void pgb_static(const char *image_name, abi_ulong orig_loaddr, +> { +> uintptr_t loaddr = orig_loaddr; +> uintptr_t hiaddr = orig_hiaddr; +> + uintptr_t offset = 0; +> uintptr_t addr; +> +> if (hiaddr != orig_hiaddr) { +> @@ -2234,18 +2235,19 @@ static void pgb_static(const char *image_name, abi_ulong orig_loaddr, +> if (ARM_COMMPAGE) { +> /* +> * Extend the allocation to include the commpage. +> - * For a 64-bit host, this is just 4GiB; for a 32-bit host, +> - * the address arithmetic will wrap around, but the difference +> - * will produce the correct allocation size. +> + * For a 64-bit host, this is just 4GiB; for a 32-bit host we +> + * need to ensure there is space bellow the guest_base so we +> + * can map the commpage in the place needed when the address +> + * arithmetic wraps around. +> */ +> if (sizeof(uintptr_t) == 8 || loaddr >= 0x80000000u) { +> hiaddr = (uintptr_t)4 << 30; +> } else { +> - loaddr = ARM_COMMPAGE & -align; +> + offset = (128 * KiB); +> } +> } +> +> - addr = pgb_find_hole(loaddr, hiaddr - loaddr, align); +> + addr = pgb_find_hole(loaddr, hiaddr - loaddr, align, offset); +> if (addr == -1) { +> /* +> * If ARM_COMMPAGE, there *might* be a non-consecutive allocation +> @@ -2280,7 +2282,7 @@ static void pgb_dynamic(const char *image_name, long align) +> * just above that, and maximises the positive guest addresses. +> */ +> commpage = ARM_COMMPAGE & -align; +> - addr = pgb_find_hole(commpage, -commpage, align); +> + addr = pgb_find_hole(commpage, -commpage, align, 0); +> assert(addr != -1); +> guest_base = addr; +> } +> -- +> 2.20.1 +> +> + + +сре, 27. мај 2020. у 14:05 Aleksandar Markovic +<email address hidden> је написао/ла: +> +> сре, 27. мај 2020. у 12:07 Alex Bennée <email address hidden> је написао/ла: +> > +> > We rely on the pointer to wrap when accessing the high address of the +> > COMMPAGE so it lands somewhere reasonable. However on 32 bit hosts we +> > cannot afford just to map the entire 4gb address range. The old mmap +> > trial and error code handled this by just checking we could map both +> > the guest_base and the computed COMMPAGE address. +> > +> > We can't just manipulate loadaddr to get what we want so we introduce +> > an offset which pgb_find_hole can apply when looking for a gap for +> > guest_base that ensures there is space left to map the COMMPAGE +> > afterwards. +> > +> > This is arguably a little inefficient for the one 32 bit +> > value (kuser_helper_version) we need to keep there given all the +> > actual code entries are picked up during the translation phase. +> > +> > Fixes: ee94743034b +> > Bug: https://bugs.launchpad.net/qemu/+bug/1880225 +> +> For the scenario in this bug report, for today's master, before and after +> applying this patch: +> + +I am not sure how significant is this info, but I executed the test +without applying patch 1/3, so only 2/3 was applied in the case +"AFTER". + +Aleksandar + +> BEFORE: +> +> $ ~/Build/qemu-master/build-gcc/arm-linux-user/qemu-arm ./toupper_string-arm +> qemu-arm: /home/rtrk/Build/qemu-master/linux-user/elfload.c:2294: +> probe_guest_base: Assertion `have_guest_base' failed. +> Aborted +> +> AFTER: +> +> $ ~/Build/qemu-master/build-gcc/arm-linux-user/qemu-arm ./toupper_string-arm +> CONTROL RESULT: (toupper_string) +> nwlrbbmqbhcdarz owkkyhiddqscdxr jmowfrxsjybldbe fsarcbynecdyggx xpklorellnmpapq +> NWLRBBMQBHCDARZ OWKKYHIDDQSCDXR JMOWFRXSJYBLDBE FSARCBYNECDYGGX XPKLORELLNMPAPQ +> +> So, it works in my test bed. +> +> Tested-by: Aleksandar Markovic <email address hidden> +> +> > Cc: Bug 1880225 <email address hidden> +> > Signed-off-by: Alex Bennée <email address hidden> +> > Cc: Richard Henderson <email address hidden> +> > Cc: Peter Maydell <email address hidden> +> > --- +> > linux-user/elfload.c | 18 ++++++++++-------- +> > 1 file changed, 10 insertions(+), 8 deletions(-) +> > +> > diff --git a/linux-user/elfload.c b/linux-user/elfload.c +> > index d6027867a1a..31defce95b5 100644 +> > --- a/linux-user/elfload.c +> > +++ b/linux-user/elfload.c +> > @@ -2145,7 +2145,7 @@ static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, lon +> > +> > /* Return value for guest_base, or -1 if no hole found. */ +> > static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, +> > - long align) +> > + long align, uintptr_t offset) +> > { +> > GSList *maps, *iter; +> > uintptr_t this_start, this_end, next_start, brk; +> > @@ -2171,7 +2171,7 @@ static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, +> > +> > this_end = ((MapInfo *)iter->data)->start; +> > next_start = ((MapInfo *)iter->data)->end; +> > - align_start = ROUND_UP(this_start, align); +> > + align_start = ROUND_UP(this_start + offset, align); +> > +> > /* Skip holes that are too small. */ +> > if (align_start >= this_end) { +> > @@ -2221,6 +2221,7 @@ static void pgb_static(const char *image_name, abi_ulong orig_loaddr, +> > { +> > uintptr_t loaddr = orig_loaddr; +> > uintptr_t hiaddr = orig_hiaddr; +> > + uintptr_t offset = 0; +> > uintptr_t addr; +> > +> > if (hiaddr != orig_hiaddr) { +> > @@ -2234,18 +2235,19 @@ static void pgb_static(const char *image_name, abi_ulong orig_loaddr, +> > if (ARM_COMMPAGE) { +> > /* +> > * Extend the allocation to include the commpage. +> > - * For a 64-bit host, this is just 4GiB; for a 32-bit host, +> > - * the address arithmetic will wrap around, but the difference +> > - * will produce the correct allocation size. +> > + * For a 64-bit host, this is just 4GiB; for a 32-bit host we +> > + * need to ensure there is space bellow the guest_base so we +> > + * can map the commpage in the place needed when the address +> > + * arithmetic wraps around. +> > */ +> > if (sizeof(uintptr_t) == 8 || loaddr >= 0x80000000u) { +> > hiaddr = (uintptr_t)4 << 30; +> > } else { +> > - loaddr = ARM_COMMPAGE & -align; +> > + offset = (128 * KiB); +> > } +> > } +> > +> > - addr = pgb_find_hole(loaddr, hiaddr - loaddr, align); +> > + addr = pgb_find_hole(loaddr, hiaddr - loaddr, align, offset); +> > if (addr == -1) { +> > /* +> > * If ARM_COMMPAGE, there *might* be a non-consecutive allocation +> > @@ -2280,7 +2282,7 @@ static void pgb_dynamic(const char *image_name, long align) +> > * just above that, and maximises the positive guest addresses. +> > */ +> > commpage = ARM_COMMPAGE & -align; +> > - addr = pgb_find_hole(commpage, -commpage, align); +> > + addr = pgb_find_hole(commpage, -commpage, align, 0); +> > assert(addr != -1); +> > guest_base = addr; +> > } +> > -- +> > 2.20.1 +> > +> > + + + +Aleksandar Markovic <email address hidden> writes: + +> сре, 27. мај 2020. у 14:05 Aleksandar Markovic +> <email address hidden> је написао/ла: +>> +>> сре, 27. мај 2020. у 12:07 Alex Bennée <email address hidden> је написао/ла: +>> > +>> > We rely on the pointer to wrap when accessing the high address of the +>> > COMMPAGE so it lands somewhere reasonable. However on 32 bit hosts we +>> > cannot afford just to map the entire 4gb address range. The old mmap +>> > trial and error code handled this by just checking we could map both +>> > the guest_base and the computed COMMPAGE address. +>> > +>> > We can't just manipulate loadaddr to get what we want so we introduce +>> > an offset which pgb_find_hole can apply when looking for a gap for +>> > guest_base that ensures there is space left to map the COMMPAGE +>> > afterwards. +>> > +>> > This is arguably a little inefficient for the one 32 bit +>> > value (kuser_helper_version) we need to keep there given all the +>> > actual code entries are picked up during the translation phase. +>> > +>> > Fixes: ee94743034b +>> > Bug: https://bugs.launchpad.net/qemu/+bug/1880225 +>> +>> For the scenario in this bug report, for today's master, before and after +>> applying this patch: +>> +> +> I am not sure how significant is this info, but I executed the test +> without applying patch 1/3, so only 2/3 was applied in the case +> "AFTER". + +That is expected. The other fix only affects binaries run inside a +/proc-less chroot and the final patch is a test case for COMMPAGE +support. + +-- +Alex Bennée + + + +Richard Henderson <email address hidden> writes: + +> On 5/27/20 3:05 AM, Alex Bennée wrote: +>> @@ -2145,7 +2145,7 @@ static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, lon +>> +>> /* Return value for guest_base, or -1 if no hole found. */ +>> static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, +>> - long align) +>> + long align, uintptr_t offset) +>> { +>> GSList *maps, *iter; +>> uintptr_t this_start, this_end, next_start, brk; +>> @@ -2171,7 +2171,7 @@ static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, +>> +>> this_end = ((MapInfo *)iter->data)->start; +>> next_start = ((MapInfo *)iter->data)->end; +>> - align_start = ROUND_UP(this_start, align); +>> + align_start = ROUND_UP(this_start + offset, align); +>> +>> /* Skip holes that are too small. */ +> +> I suppose offset is supposed to mean we start from -offset? + +Well guest_base will start higher meaning we have space for the +commpage beneath it. + +> You didn't update +> pgb_find_hole_fallback. + +Fixed. + +> +>> - loaddr = ARM_COMMPAGE & -align; +>> + offset = (128 * KiB); +> +> Why 128K? Surely this should be an expression against ARM_COMMPAGE. + +In theory: + + offset = -(ARM_COMMPAGE & -align); + +should do the trick but I found it failed every now and again. +Frustratingly putting printfs in made it go away so in frustration I +just upped the offset until it stopped happening. + +I do kinda wish rr worked on i386 :-/ + + +> +> +> r~ + + +-- +Alex Bennée + + + +Alex Bennée <email address hidden> writes: + +> Richard Henderson <email address hidden> writes: +> +>> On 5/27/20 3:05 AM, Alex Bennée wrote: +>>> @@ -2145,7 +2145,7 @@ static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, lon +>>> +>>> /* Return value for guest_base, or -1 if no hole found. */ +>>> static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, +>>> - long align) +>>> + long align, uintptr_t offset) +>>> { +>>> GSList *maps, *iter; +>>> uintptr_t this_start, this_end, next_start, brk; +>>> @@ -2171,7 +2171,7 @@ static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, +>>> +>>> this_end = ((MapInfo *)iter->data)->start; +>>> next_start = ((MapInfo *)iter->data)->end; +>>> - align_start = ROUND_UP(this_start, align); +>>> + align_start = ROUND_UP(this_start + offset, align); +>>> +>>> /* Skip holes that are too small. */ +>> +>> I suppose offset is supposed to mean we start from -offset? +> +> Well guest_base will start higher meaning we have space for the +> commpage beneath it. +> +>> You didn't update +>> pgb_find_hole_fallback. +> +> Fixed. +> +>> +>>> - loaddr = ARM_COMMPAGE & -align; +>>> + offset = (128 * KiB); +>> +>> Why 128K? Surely this should be an expression against ARM_COMMPAGE. +> +> In theory: +> +> offset = -(ARM_COMMPAGE & -align); +> +> should do the trick but I found it failed every now and again. +> Frustratingly putting printfs in made it go away so in frustration I +> just upped the offset until it stopped happening. +> +> I do kinda wish rr worked on i386 :-/ + +Ahh all I needed was a MAP_FIXED for init_commpage + +-- +Alex Bennée + + +We rely on the pointer to wrap when accessing the high address of the +COMMPAGE so it lands somewhere reasonable. However on 32 bit hosts we +cannot afford just to map the entire 4gb address range. The old mmap +trial and error code handled this by just checking we could map both +the guest_base and the computed COMMPAGE address. + +We can't just manipulate loadaddr to get what we want so we introduce +an offset which pgb_find_hole can apply when looking for a gap for +guest_base that ensures there is space left to map the COMMPAGE +afterwards. + +This is arguably a little inefficient for the one 32 bit +value (kuser_helper_version) we need to keep there given all the +actual code entries are picked up during the translation phase. + +Fixes: ee94743034b +Bug: https://bugs.launchpad.net/qemu/+bug/1880225 +Cc: Bug 1880225 <email address hidden> +Signed-off-by: Alex Bennée <email address hidden> +Tested-by: Aleksandar Markovic <email address hidden> +Cc: Richard Henderson <email address hidden> +Cc: Peter Maydell <email address hidden> +Message-Id: <email address hidden> + +--- +v3 + - base offset on ARM_COMMPAGE + - ensure we MAP_FIXED the commpage + - support offset in pgd_find_hole_fallback +--- + linux-user/elfload.c | 31 +++++++++++++++++-------------- + 1 file changed, 17 insertions(+), 14 deletions(-) + +diff --git a/linux-user/elfload.c b/linux-user/elfload.c +index 475d243f3bd..b5cb21384a1 100644 +--- a/linux-user/elfload.c ++++ b/linux-user/elfload.c +@@ -389,7 +389,7 @@ static bool init_guest_commpage(void) + { + void *want = g2h(ARM_COMMPAGE & -qemu_host_page_size); + void *addr = mmap(want, qemu_host_page_size, PROT_READ | PROT_WRITE, +- MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); ++ MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0); + + if (addr == MAP_FAILED) { + perror("Allocating guest commpage"); +@@ -2113,7 +2113,8 @@ static void pgb_have_guest_base(const char *image_name, abi_ulong guest_loaddr, + * only dumbly iterate up the host address space seeing if the + * allocation would work. + */ +-static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, long align) ++static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, ++ long align, uintptr_t offset) + { + uintptr_t base; + +@@ -2123,7 +2124,7 @@ static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, lon + while (true) { + uintptr_t align_start, end; + align_start = ROUND_UP(base, align); +- end = align_start + guest_size; ++ end = align_start + guest_size + offset; + + /* if brk is anywhere in the range give ourselves some room to grow. */ + if (align_start <= brk && brk < end) { +@@ -2138,7 +2139,7 @@ static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, lon + PROT_NONE, flags, -1, 0); + if (mmap_start != MAP_FAILED) { + munmap((void *) align_start, guest_size); +- return (uintptr_t) mmap_start; ++ return (uintptr_t) mmap_start + offset; + } + base += qemu_host_page_size; + } +@@ -2147,7 +2148,7 @@ static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, lon + + /* Return value for guest_base, or -1 if no hole found. */ + static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, +- long align) ++ long align, uintptr_t offset) + { + GSList *maps, *iter; + uintptr_t this_start, this_end, next_start, brk; +@@ -2161,7 +2162,7 @@ static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, + brk = (uintptr_t)sbrk(0); + + if (!maps) { +- return pgd_find_hole_fallback(guest_size, brk, align); ++ return pgd_find_hole_fallback(guest_size, brk, align, offset); + } + + /* The first hole is before the first map entry. */ +@@ -2173,7 +2174,7 @@ static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, + + this_end = ((MapInfo *)iter->data)->start; + next_start = ((MapInfo *)iter->data)->end; +- align_start = ROUND_UP(this_start, align); ++ align_start = ROUND_UP(this_start + offset, align); + + /* Skip holes that are too small. */ + if (align_start >= this_end) { +@@ -2223,6 +2224,7 @@ static void pgb_static(const char *image_name, abi_ulong orig_loaddr, + { + uintptr_t loaddr = orig_loaddr; + uintptr_t hiaddr = orig_hiaddr; ++ uintptr_t offset = 0; + uintptr_t addr; + + if (hiaddr != orig_hiaddr) { +@@ -2236,18 +2238,19 @@ static void pgb_static(const char *image_name, abi_ulong orig_loaddr, + if (ARM_COMMPAGE) { + /* + * Extend the allocation to include the commpage. +- * For a 64-bit host, this is just 4GiB; for a 32-bit host, +- * the address arithmetic will wrap around, but the difference +- * will produce the correct allocation size. ++ * For a 64-bit host, this is just 4GiB; for a 32-bit host we ++ * need to ensure there is space bellow the guest_base so we ++ * can map the commpage in the place needed when the address ++ * arithmetic wraps around. + */ + if (sizeof(uintptr_t) == 8 || loaddr >= 0x80000000u) { +- hiaddr = (uintptr_t)4 << 30; ++ hiaddr = (uintptr_t) 4 << 30; + } else { +- loaddr = ARM_COMMPAGE & -align; ++ offset = -(ARM_COMMPAGE & -align); + } + } + +- addr = pgb_find_hole(loaddr, hiaddr - loaddr, align); ++ addr = pgb_find_hole(loaddr, hiaddr - loaddr, align, offset); + if (addr == -1) { + /* + * If ARM_COMMPAGE, there *might* be a non-consecutive allocation +@@ -2282,7 +2285,7 @@ static void pgb_dynamic(const char *image_name, long align) + * just above that, and maximises the positive guest addresses. + */ + commpage = ARM_COMMPAGE & -align; +- addr = pgb_find_hole(commpage, -commpage, align); ++ addr = pgb_find_hole(commpage, -commpage, align, 0); + assert(addr != -1); + guest_base = addr; + } +-- +2.20.1 + + + +We rely on the pointer to wrap when accessing the high address of the +COMMPAGE so it lands somewhere reasonable. However on 32 bit hosts we +cannot afford just to map the entire 4gb address range. The old mmap +trial and error code handled this by just checking we could map both +the guest_base and the computed COMMPAGE address. + +We can't just manipulate loadaddr to get what we want so we introduce +an offset which pgb_find_hole can apply when looking for a gap for +guest_base that ensures there is space left to map the COMMPAGE +afterwards. + +This is arguably a little inefficient for the one 32 bit +value (kuser_helper_version) we need to keep there given all the +actual code entries are picked up during the translation phase. + +Fixes: ee94743034b +Bug: https://bugs.launchpad.net/qemu/+bug/1880225 +Cc: Bug 1880225 <email address hidden> +Signed-off-by: Alex Bennée <email address hidden> +Tested-by: Aleksandar Markovic <email address hidden> +Cc: Richard Henderson <email address hidden> +Cc: Peter Maydell <email address hidden> +Message-Id: <email address hidden> + +diff --git a/linux-user/elfload.c b/linux-user/elfload.c +index 475d243f3bd..b5cb21384a1 100644 +--- a/linux-user/elfload.c ++++ b/linux-user/elfload.c +@@ -389,7 +389,7 @@ static bool init_guest_commpage(void) + { + void *want = g2h(ARM_COMMPAGE & -qemu_host_page_size); + void *addr = mmap(want, qemu_host_page_size, PROT_READ | PROT_WRITE, +- MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); ++ MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0); + + if (addr == MAP_FAILED) { + perror("Allocating guest commpage"); +@@ -2113,7 +2113,8 @@ static void pgb_have_guest_base(const char *image_name, abi_ulong guest_loaddr, + * only dumbly iterate up the host address space seeing if the + * allocation would work. + */ +-static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, long align) ++static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, ++ long align, uintptr_t offset) + { + uintptr_t base; + +@@ -2123,7 +2124,7 @@ static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, lon + while (true) { + uintptr_t align_start, end; + align_start = ROUND_UP(base, align); +- end = align_start + guest_size; ++ end = align_start + guest_size + offset; + + /* if brk is anywhere in the range give ourselves some room to grow. */ + if (align_start <= brk && brk < end) { +@@ -2138,7 +2139,7 @@ static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, lon + PROT_NONE, flags, -1, 0); + if (mmap_start != MAP_FAILED) { + munmap((void *) align_start, guest_size); +- return (uintptr_t) mmap_start; ++ return (uintptr_t) mmap_start + offset; + } + base += qemu_host_page_size; + } +@@ -2147,7 +2148,7 @@ static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, lon + + /* Return value for guest_base, or -1 if no hole found. */ + static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, +- long align) ++ long align, uintptr_t offset) + { + GSList *maps, *iter; + uintptr_t this_start, this_end, next_start, brk; +@@ -2161,7 +2162,7 @@ static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, + brk = (uintptr_t)sbrk(0); + + if (!maps) { +- return pgd_find_hole_fallback(guest_size, brk, align); ++ return pgd_find_hole_fallback(guest_size, brk, align, offset); + } + + /* The first hole is before the first map entry. */ +@@ -2173,7 +2174,7 @@ static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size, + + this_end = ((MapInfo *)iter->data)->start; + next_start = ((MapInfo *)iter->data)->end; +- align_start = ROUND_UP(this_start, align); ++ align_start = ROUND_UP(this_start + offset, align); + + /* Skip holes that are too small. */ + if (align_start >= this_end) { +@@ -2223,6 +2224,7 @@ static void pgb_static(const char *image_name, abi_ulong orig_loaddr, + { + uintptr_t loaddr = orig_loaddr; + uintptr_t hiaddr = orig_hiaddr; ++ uintptr_t offset = 0; + uintptr_t addr; + + if (hiaddr != orig_hiaddr) { +@@ -2236,18 +2238,19 @@ static void pgb_static(const char *image_name, abi_ulong orig_loaddr, + if (ARM_COMMPAGE) { + /* + * Extend the allocation to include the commpage. +- * For a 64-bit host, this is just 4GiB; for a 32-bit host, +- * the address arithmetic will wrap around, but the difference +- * will produce the correct allocation size. ++ * For a 64-bit host, this is just 4GiB; for a 32-bit host we ++ * need to ensure there is space bellow the guest_base so we ++ * can map the commpage in the place needed when the address ++ * arithmetic wraps around. + */ + if (sizeof(uintptr_t) == 8 || loaddr >= 0x80000000u) { +- hiaddr = (uintptr_t)4 << 30; ++ hiaddr = (uintptr_t) 4 << 30; + } else { +- loaddr = ARM_COMMPAGE & -align; ++ offset = -(ARM_COMMPAGE & -align); + } + } + +- addr = pgb_find_hole(loaddr, hiaddr - loaddr, align); ++ addr = pgb_find_hole(loaddr, hiaddr - loaddr, align, offset); + if (addr == -1) { + /* + * If ARM_COMMPAGE, there *might* be a non-consecutive allocation +@@ -2282,7 +2285,7 @@ static void pgb_dynamic(const char *image_name, long align) + * just above that, and maximises the positive guest addresses. + */ + commpage = ARM_COMMPAGE & -align; +- addr = pgb_find_hole(commpage, -commpage, align); ++ addr = pgb_find_hole(commpage, -commpage, align, 0); + assert(addr != -1); + guest_base = addr; + } +-- +2.20.1 + + + +Fixed here: +https://git.qemu.org/?p=qemu.git;a=commitdiff;h=5c3e87f345ac93de9260f + |