Bug 210268 - darwin merge broke mmap2/mprotect behavior on linux
Summary: darwin merge broke mmap2/mprotect behavior on linux
Status: RESOLVED DUPLICATE of bug 205541
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: blocking3.5.1
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-12 02:38 UTC by Dan Kegel
Modified: 2009-10-18 05:37 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dan Kegel 2009-10-12 02:38:39 UTC
Version:            (using Devel)
OS:                Linux
Installed from:    Compiled sources

On x86 linux (ubuntu 9.04), the test program

#include <sys/mman.h>
#include <string.h>
#include <assert.h>
int main(int argc, char **argv)
{
    char *buf = (char *)0x7f000000;
    char *p = mmap(buf, 1114112, PROT_NONE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0);
    int err;
    assert(p == buf);
    err = mprotect(buf, 65536, PROT_READ|PROT_WRITE);
    assert(err == 0);
    strcpy(p, "hello, mmap");
}

runs fine in valgrind right up to 
"r10156 | njn | 2009-05-27 18:53:07 -0700 (Wed, 27 May 2009) | 7 lines
Merge the DARWIN branch onto the trunk."

Afterwards, valgrind complains

Invalid write of size 1
 at memcpy (mc_replace_strmem.c:482)
 by main (foo.c:13)
 Address 0x7f00000b is not stack'd, malloc'd or (recently) free'd

It's not surprising that the darwin merge broke
something.  It *is* somewhat surprising that nobody
noticed until now.  Maybe that sequence is rare...

( Previously filed as 
http://bugs.winehq.org/show_bug.cgi?id=20303, but turned
out to be a problem in valgrind, not wine.  Fortunately,
strace revealed the test program fairly directly. )
Comment 1 Jakub Jelinek 2009-10-12 17:12:18 UTC
The problem is in the darwin memcheck/mc_main.c changes, in particular mc_new_mem_startup/mc_new_mem_mmap now doing MC_(make_mem_noaccess)(a, len);
for PROT_NONE mappings instead of MC_(make_mem_defined)(a, len);
ATM memcheck ignores change_mem_mprotect, so if something is mapped as PROT_NONE initially, it is tracked as noaccess even if it is latter on changed with mprotect.

Either this change should be reverted (or guarded with darwin define only, or limited to Addr 0 only), or mc_main.c needs to be taught to track also mprotect.  Given the comment why it doesn't track mprotect I think making it noaccess on PROT_NONE mprotect and defined on other mprotect is not desirable, so perhaps just making it defined on != PROT_NONE mprotect or walking the range and changing any noaccess ranges in it to defined.
Comment 2 Nicholas Nethercote 2009-10-12 23:32:12 UTC
Thanks for the diagnosis, Jakub.
Comment 3 Dan Kegel 2009-10-18 05:37:11 UTC
Looks like John Reiser got there first; see
bug 205541.  He even submitted a possible patch.
Comment 4 Dan Kegel 2009-10-18 05:37:29 UTC

*** This bug has been marked as a duplicate of bug 205541 ***