https://bugs.kde.org/show_bug.cgi?id=514297

--- Comment #16 from [email protected] ---
Created attachment 190171
  --> https://bugs.kde.org/attachment.cgi?id=190171&action=edit
wip patch

Hi Mark,  I'm attaching a new patch, it's still a WIP, but I'd appreciate your
thoughts.

I've created a new function  VG_(am_notify_madv_guard_install) initially by
copying ML_(notify_core_and_tool_of_mprotect)().  I've commented out the tool
and debuginfo modules notification, so that it actually only notifies the core
for now.  That seems sufficient for my testcase.  I've spent some time trying
to understand how the tool and debuginfo callbacks work in
ML_(notify_core_and_tool_of_mprotect)(), and I suspect this new
VG_(am_notify_madv_guard_install) will need uncommenting that part again so
that it offers at least empty plugs for valgrind tools.  I've kept translations
discarding in place, since I think it's as relevant as it is for the mprotect
case.

The hasGuardPages boolean was added to the NSegment struct as agreed.  This way
VMAs containing guard pages are flagged.  I've checked that size of VMAs of
"normal programs" is typically up to hundreds of pages.  I've noticed that
memcheck has a detailed map of whole the memory via the V and A bits, so guard
pages could in theory be tracked there as well instead of the aspacem. But
since each page has thousands of addresses, it seems more efficient to keep the
guard page info in aspacemgr then it would be in memcheck's A/V bitmap
representation.

To actually check for the guarded addresses, the aspacemgr's is_valid_for()
function was modified.  In case it comes across a VMA that is flagged with
hasGuardPages, it will call newly created belongs_to_guard_page() function. 
This function needs more work, certainly because it (still) repeatedly opens
and closes /proc/self/pagemap.  But regarding caching the content of this file,
I tend to think that it is not needed: It's not a disk backed file, and reading
from it is about as fast as reading from a cache would be, I suspect.  Note
that with lseek we quicky jump right to the page info we are interested in, so
that it's pretty fast. 

The patch also contains the reproducer program, I'll drop that later.

Does this look like going in the right direction?  Any thoughts?

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to