Control: found -1 2016.10.0-1
Control: fixed -1 2018.03.0-1
On Tue, Jan 05, 2021 at 09:42:35PM +0100, Uwe Kleine-König wrote:
> On 1/2/21 8:34 PM, Gergely Peli wrote:
> > Package: memtool
> > Severity: normal
> > Tags: upstream patch
> >
> > Hello,
> >
> > I tried memtool on a 32 bit ARM system, and the supposedly 64 bit reads
> > with the "-q" option are done by 2 separate 32 bit "ldr" instructions.
> > Adding a volatile qualifier to the pointers accessing the memory could
> > convince gcc to generate a single "ldrd" instruction instead, which uses
> > a 64 bit memory transfer. See below a proposed patch. Adding volatile to all
> > 4 cases may be an overkill, but can't hurt. Cheers,
> >
> > Gergely Peli
> >
> > ========
> >
> > --- memtool-2016.10.0.orig/memtool.c
> > +++ memtool-2016.10.0/memtool.c
> > @@ -152,24 +152,24 @@ static int memory_display(const void *ad
> > for (i = 0; i < linebytes; i += width) {
> > if (width == 8) {
> > uint64_t res;
> > - res = (*uqp++ = *((uint64_t *)addr));
> > + res = (*uqp++ = *((volatile uint64_t
> > *)addr));
>
> In this code location it doesn't matter if two ldrs instead of a single ldrd
> is used. This is only the code printing the data from the buffer that holds
> the read data. If memtools behaves wrong the problem is likely in mmap_read.In a private followup conversation it became obvious that there is indeed a culprit but only in oldstable with version 2016.10.0-1 where memory_display operates directly on the mmap'd memory. With changes introduced in 2018.03.0 the problem is gone. (It's not entirely clear why though, maybe there is also some luck involved because the compiler behaviour changed and we might need some volatiles in mmap_read (and mmap_write) for correctness). Anyhow, I checked memtool 2018.03.0 on armhf and there a 64 bit read (vldr.64, not ldrd) is used. So I marked 2018.03.0-1 as fixed. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
