On Mon, Oct 29, 2018 at 05:00:36PM +0100, Jakub Jelinek wrote:
> On Mon, Oct 29, 2018 at 10:38:11AM -0500, Bill Seurer wrote:
> > > I'm still wondering what didn't work with 41 bits? AFAICS, due to
> > > highshadow=highmem-offset and lowshadow=low+offset, and the existence of a
> > > non-empty shadow-gap, offset must be minimum(vbits)-3 (vbits being one of
> > > the above numbers). Why would 41 not work for the other vbit setting?
> > > It would lead to a large shadow gap, but so? If a shadow-gap isn't
> > > necessary then minimum(vbits)-2 would also work.
> >
> > 41 was what the value previously was and it did not work (IIRC with 47 bit
> > VMA kernels which were "new" at the time) thus the change.
>
> What we'd like to see is details or URL with those why it didn't work with
> 47 bit. E.g. x86_64 uses much lower shadow offset (0x7fff8000), asan has
> support for multiple shadow areas, gaps and normal areas.
> I think ppc64 binaries are usually using 0x10000000 base, so that isn't a
> problem, so is there a conflict with the default dynamic linker placement or
> default placement of shared libraries when using the 47-bit VMA?
> What do you get with the 41-bit shadow offset and ASAN_OPTIONS=verbosity=1
> on 47-bit VMA that e.g. kMidMemBeg/kMidMemEnd couldn't solve?
Thanks to Bill for debugging last night; my understanding is that the
1UL<<41 to 1UL<<44 shadow offset change was an attempt to fix the issue that
Martin fixed on the gcc side (and in llvm too) with changing PPC64
kAllocatorSpace from 0xa0000000000ULL to ~(uptr)0. Sowe can now safely go
back to 1UL<<41 shadow offset, which will work also with 44-bit VA.
That setting is known to work with 44-bit, 46-bit and 47-bit virtual address
space, we don't know if there are any kernels out there with the higher
address space sizes the recent 4.13+ and 4.18+ kernel sources suggest
(49-bit and 52-bit). Guess we'll need to talk to some kernel people.
Anyway, for the gcc backport, I'd suggest to revert the 41->44 changes
and don't bump libasan soname.
The patch set LGTM with those changes.
Jakub
