|
From: Konstantin S. <kon...@gm...> - 2009-05-22 11:45:31
|
On Fri, May 22, 2009 at 2:27 PM, Julian Seward <js...@ac...> wrote:
>
> Konstantin,
>
> (w/ apologies if you did already ..)
>
> Could you give some background to explain why you need to mark a
> particular memory access to a location as OK, but not others?
> Am kinda curious to see a bit the bigger picture.
Many people write code with strlen-like hacks, i.e. they access memory
out of bounds (or uninitialized memory) and know this is correct.
Example (from my initial message):
int *p = (int*)malloc(100);
// p is 4-aligned and not initialized
if (...) {
p[0] = '\0';
} else {
p[0] = 'h';
p[1] = 't';
p[2] = 't';
p[3] = 'p';
}
...
// here we read 4 first bytes of p.
// bytes p[1],p[2],p[3] may not be initialized, but this is safe, because
// in this case p[0] will not be equal to 'h'.
int first_four_bytes_of_p = ANNOTATE_SAFE_READ(*(int*)p);
if (first_four_bytes_of_p == combine_four_chars_into_int("http")) ...
And some of these warnings can't be easily suppressed with valgrind
suppressions (you know why, otherwise you wouldn't intercept strlen,
you would just suppress it).
Even if some warning can be suppressed with valgrind suppressions,
many people want to have the annotation in the code, not in the
external file.
With ThreadSanitizer the in-code annotations proved to be very useful.
--kcc
>
> J
>
> On Friday 22 May 2009, Konstantin Serebryany wrote:
>> Hi Nick,
>>
>> Here is my attempt for a patch and a unittest.
>> The first thing you would probably ask is how does this affect the
>> performance of Memcheck.
>> Would you suggest a benchmark to try?
>> Other comments?
>>
>> Thanks,
>>
>> --kcc
>>
>> % g++ -g -DHAVE_IGNORE_ANNOTATIONS memcheck_annotations.cc -I
>> $HOME/valgrind/ann/inst/include &&
>> $HOME/valgrind/ann/inst/bin/valgrind -q ./a.out
>> % g++ -g memcheck_annotations.cc -I $HOME/valgrind/ann/inst/include
>> && $HOME/valgrind/ann/inst/bin/valgrind -q ./a.out 2>&1 | head
>> ==16258== Invalid write of size 1
>> ==16258== at 0x4006D9: int test_memcheck_annotations<signed char>()
>> (memcheck_annotations.cc:16)
>> ==16258== by 0x4006A4: main (memcheck_annotations.cc:31)
>> ==16258== Address 0x58f603a is 0 bytes after a block of size 10 alloc'd
>> ==16258== at 0x4C1C87C: operator new[](unsigned long)
>> (vg_replace_malloc.c:245)
>> ==16258== by 0x4006CC: int test_memcheck_annotations<signed char>()
>> (memcheck_annotations.cc:13)
>> ==16258== by 0x4006A4: main (memcheck_annotations.cc:31)
>> ==16258==
>> ==16258== Invalid read of size 1
>> ==16258== at 0x4006E4: int test_memcheck_annotations<signed char>()
>> (memcheck_annotations.cc:19)
>> %
>>
>>
>>
>>
>>
>>
>> On Wed, Apr 8, 2009 at 6:25 PM, Konstantin Serebryany
>>
>> <kon...@gm...> wrote:
>> > On Wed, Apr 8, 2009 at 2:07 PM, Nicholas Nethercote
>> >
>> > <n.n...@gm...> wrote:
>> >> On Wed, Apr 8, 2009 at 2:20 AM, Konstantin Serebryany
>> >>
>> >> <kon...@gm...> wrote:
>> >>> Memcheck has client requests such as VALGRIND_MAKE_MEM_DEFINED(mem,
>> >>> size). These requests mark memory as initialized (i.e. safe to read).
>> >>> But there is no annotation that marks a specific memory access as safe.
>> >>> Is that not included intentionally? If yes, what was the reason? If no,
>> >>> do you mind to include such request(s)?
>> >>
>> >> I think the reason is that nobody has required them until now.
>> >>
>> >> I've previously thought about them in one context, involving more
>> >> aggressive checking by Memcheck of a program. Imagine you have a data
>> >> structure that should only be accessed through a small number of
>> >> access functions. You could mark the data structure as unaddressable,
>> >> and then wrap the accesses within the access functions with such
>> >> annotations. That way you would immediately know if any other part of
>> >> your program accessed the data structure not via those access
>> >> functions, which could be due to (a) failure to observe the access
>> >> function discipline, or (b) corrupted pointers or similar.
>> >>
>> >> I thought about doing this by marking the memory as
>> >> addressable-and-defined, doing the read/write, then marking it
>> >> unaddressable again, but your approach is more efficient. So I can see
>> >> these annotations would be useful. Here's an attempt at what they
>> >> would look like:
>> >>
>> >> // Memcheck normally checks that you do not read unaddressable or
>> >> // addressable-but-undefined memory. This wrapper causes that
>> >> // check to be omitted.
>> >> VALGRIND_UNCHECKED_MEM_READ(mem)
>> >>
>> >> // Memcheck normally checks that you do not write unaddressable memory.
>> >> // This wrapper causes that check to be omitted.
>> >> VALGRIND_UNCHECKED_MEM_WRITE(mem)
>> >
>> > In addition to these, we may want to have a BEGIN/END form, for the
>> > cases when the access happens in a routine which we can not annotate
>> > or which may be used in other contexts.
>> > VALGRIND_IGNORE_READS_BEGIN()
>> > external_function(pointer_to_uninitialized_but_safe_to_use_memory)
>> > VALGRIND_IGNORE_READS_END()
>> >
>> >>> Example:
>> >>>
>> >>> int *p = (int*)malloc(100);
>> >>> // p is 4-aligned and not initialized
>> >>
>> >> Not quite. p is 4-aligned and initialized. *p is unitialized.
>> >
>> > ah, yes.
>> >
>> > --kcc
>
>
>
|