| Message ID | 20190328143003.16702-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [for-4.1] target/arm: Stop using variable length array in dc_zva | expand |
Le jeu. 28 mars 2019 15:30, Peter Maydell <peter.maydell@linaro.org> a écrit : > Currently the dc_zva helper function uses a variable length > array. In fact we know (as the comment above remarks) that > the length of this array is bounded because the architecture > limits the block size and QEMU limits the target page size. > Use a fixed array size and assert that we don't run off it. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > A small move in the direction of "avoid using variable length > arrays in QEMU"... > > target/arm/helper.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/target/arm/helper.c b/target/arm/helper.c > index a36f4b3d699..1b6225cb598 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -1,4 +1,5 @@ > #include "qemu/osdep.h" > +#include "qemu/units.h" > #include "target/arm/idau.h" > #include "trace.h" > #include "cpu.h" > @@ -12412,11 +12413,13 @@ void HELPER(dc_zva)(CPUARMState *env, uint64_t > vaddr_in) > * same QEMU executable. > */ > int maxidx = DIV_ROUND_UP(blocklen, TARGET_PAGE_SIZE); > - void *hostaddr[maxidx]; > + void *hostaddr[DIV_ROUND_UP(2 * KiB, 1 << TARGET_PAGE_BITS_MIN)]; > Or g_new()... For 2K nowadays that is fine. Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> int try, i; > unsigned mmu_idx = cpu_mmu_index(env, false); > TCGMemOpIdx oi = make_memop_idx(MO_UB, mmu_idx); > > + assert(maxidx <= sizeof(hostaddr)); > + > for (try = 0; try < 2; try++) { > > for (i = 0; i < maxidx; i++) { > -- > 2.20.1 > > > <div dir="auto"><div><br><div class="gmail_quote"><div dir="ltr">Le jeu. 28 mars 2019 15:30, Peter Maydell <<a href="mailto:peter.maydell@linaro.org">peter.maydell@linaro.org</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Currently the dc_zva helper function uses a variable length<br> array. In fact we know (as the comment above remarks) that<br> the length of this array is bounded because the architecture<br> limits the block size and QEMU limits the target page size.<br> Use a fixed array size and assert that we don't run off it.<br> <br> Signed-off-by: Peter Maydell <<a href="mailto:peter.maydell@linaro.org" target="_blank" rel="noreferrer">peter.maydell@linaro.org</a>><br> ---<br> A small move in the direction of "avoid using variable length<br> arrays in QEMU"...<br> <br> target/arm/helper.c | 5 ++++-<br> 1 file changed, 4 insertions(+), 1 deletion(-)<br> <br> diff --git a/target/arm/helper.c b/target/arm/helper.c<br> index a36f4b3d699..1b6225cb598 100644<br> --- a/target/arm/helper.c<br> +++ b/target/arm/helper.c<br> @@ -1,4 +1,5 @@<br> #include "qemu/osdep.h"<br> +#include "qemu/units.h"<br> #include "target/arm/idau.h"<br> #include "trace.h"<br> #include "cpu.h"<br> @@ -12412,11 +12413,13 @@ void HELPER(dc_zva)(CPUARMState *env, uint64_t vaddr_in)<br> * same QEMU executable.<br> */<br> int maxidx = DIV_ROUND_UP(blocklen, TARGET_PAGE_SIZE);<br> - void *hostaddr[maxidx];<br> + void *hostaddr[DIV_ROUND_UP(2 * KiB, 1 << TARGET_PAGE_BITS_MIN)];<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Or g_new()... For 2K nowadays that is fine.</div><div dir="auto"><br></div><div dir="auto">Reviewed-by: Philippe Mathieu-Daudé <<a href="mailto:f4bug@amsat.org">f4bug@amsat.org</a>><br></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> int try, i;<br> unsigned mmu_idx = cpu_mmu_index(env, false);<br> TCGMemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);<br> <br> + assert(maxidx <= sizeof(hostaddr));<br> +<br> for (try = 0; try < 2; try++) {<br> <br> for (i = 0; i < maxidx; i++) {<br> -- <br> 2.20.1<br> <br> <br> </blockquote></div></div></div>
On 3/28/19 7:30 AM, Peter Maydell wrote: > - void *hostaddr[maxidx]; > + void *hostaddr[DIV_ROUND_UP(2 * KiB, 1 << TARGET_PAGE_BITS_MIN)]; A very fancy way of writing "2". > int try, i; > unsigned mmu_idx = cpu_mmu_index(env, false); > TCGMemOpIdx oi = make_memop_idx(MO_UB, mmu_idx); > > + assert(maxidx <= sizeof(hostaddr)); ARRAY_SIZE(hostaddr). r~
On Thu, 28 Mar 2019 at 19:14, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 3/28/19 7:30 AM, Peter Maydell wrote: > > - void *hostaddr[maxidx]; > > + void *hostaddr[DIV_ROUND_UP(2 * KiB, 1 << TARGET_PAGE_BITS_MIN)]; > > A very fancy way of writing "2". Yes, but I thought this made the relationship between the constant size and what maxidx a little clearer. > > > int try, i; > > unsigned mmu_idx = cpu_mmu_index(env, false); > > TCGMemOpIdx oi = make_memop_idx(MO_UB, mmu_idx); > > > > + assert(maxidx <= sizeof(hostaddr)); > > ARRAY_SIZE(hostaddr). Oops, yes. thanks -- PMM
diff --git a/target/arm/helper.c b/target/arm/helper.c index a36f4b3d699..1b6225cb598 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -1,4 +1,5 @@ #include "qemu/osdep.h" +#include "qemu/units.h" #include "target/arm/idau.h" #include "trace.h" #include "cpu.h" @@ -12412,11 +12413,13 @@ void HELPER(dc_zva)(CPUARMState *env, uint64_t vaddr_in) * same QEMU executable. */ int maxidx = DIV_ROUND_UP(blocklen, TARGET_PAGE_SIZE); - void *hostaddr[maxidx]; + void *hostaddr[DIV_ROUND_UP(2 * KiB, 1 << TARGET_PAGE_BITS_MIN)]; int try, i; unsigned mmu_idx = cpu_mmu_index(env, false); TCGMemOpIdx oi = make_memop_idx(MO_UB, mmu_idx); + assert(maxidx <= sizeof(hostaddr)); + for (try = 0; try < 2; try++) { for (i = 0; i < maxidx; i++) {
Currently the dc_zva helper function uses a variable length array. In fact we know (as the comment above remarks) that the length of this array is bounded because the architecture limits the block size and QEMU limits the target page size. Use a fixed array size and assert that we don't run off it. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- A small move in the direction of "avoid using variable length arrays in QEMU"... target/arm/helper.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) -- 2.20.1