libsanitizer merge from upstream r175042

Konstantin Serebryany konstantin.s.serebryany@gmail.com
Fri Feb 15 08:48:00 GMT 2013


Ian, there is a question for you below.

On Fri, Feb 15, 2013 at 12:26 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Feb 15, 2013 at 11:45:15AM +0400, Konstantin Serebryany wrote:
>> On Thu, Feb 14, 2013 at 4:19 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > On Thu, Feb 14, 2013 at 03:55:47PM +0400, Konstantin Serebryany wrote:
>> >> The patch seems to work on a simple test. Let me digest it.
>> >> I am trying to understand if there are problems with it other than the
>> >> added complexity (which is what I don't like the most).
>> >
>> > Yes, it is some added complexity, but not too much, and something that can
>> > be tested regularly that it works.
>>
>> The complexity I am afraid of is not only in the code, but also at the
>> time of execution.
>> We and our users sometimes have to stare at the /proc/self/maps.
>> A mapping with 1 (ZeroBase) or 3 (default) asan sections is ok, but
>> 6 extra asan sections becomes nearly incomprehensible, at least for me.
>>
>> So, how about having kMidMemBeg as a variable, set as __asan_init.
>> Only if something is mapped around 0x003X00000000 we set it to non-zero.
>
> That is fine for me.  Note that ASAN_FIXED_MAPPING 1 might not work well,
> e.g. for shadow offset of 1ULL << 44, there the prelink library range falls
> into the low memory and thus kMidMemBeg should be 0.

Sure. ASAN_FIXED_MAPPING should be used for performance measurements
only -- this is not a release option.
(Added a more precise comment).

>
>> http://llvm-reviews.chandlerc.com/D411 (still needs some cleanup)
>
> One cleanup might be avoid calling MemoryRangeIsAvailable unnecessarily
> too many times.  Guess if you move:
>   uptr shadow_start = kLowShadowBeg;
>   if (kLowShadowBeg) shadow_start -= GetMmapGranularity();
>   uptr shadow_end = kHighShadowEnd;
>   bool shadow_avail = MemoryRangeIsAvailable(shadow_start, shadow_end);
> before the
> #if ASAN_LINUX && defined(__x86_64__) && !ASAN_FIXED_MAPPING
>   if (!MemoryRangeIsAvailable(kLowShadowBeg, kHighShadowEnd)) {
>     kMidMemBeg = kLowMemEnd < 0x3000000000ULL ? 0x3000000000ULL : 0;
>     kMidMemEnd = kLowMemEnd < 0x3000000000ULL ? 0x3fffffffffULL : 0;
>   }
> #endif
> code, you can just use shadow_avail there and later.  Every
> MemoryRangeIsAvailable reads /proc/self/maps again, right?

agree, done.

>
> Also, on ppc64 the prelink library area is:
> 0x8001000000LL to 0x8100000000LL if we want to e.g. handle flexible mapping
> there (perhaps better use 0x8000000000L as kMidMemBeg then), guess for 32-bit
> architectures it is irrelevant, there is not much
> point in using shadow offsets other than the default high one (which is high
> enough) or 0 (then you need PIE, and thus prelink info is ignored both by
> the kernel and dynamic linker).

Let's worry about ppc prelink separately.
We are not changing the shadow offset on ppc (because 7fff8000 does
not seem to help it anyway)
and no one complained so far.

>
>> Unfortunately, the test does not work if gold is the system linker.
>> Any suggestion on how to make the test work with either linker?
>
> That is the question to Ian, if gold can do that at all.

Yep.

>
> As I said before, you can try something like:
>
> #include <sys/mman.h>
>
> struct A
> {
>   A ();
>   void *ptr;
> };
>
> void *ptr;
>
> __attribute__((no_address_safety_analysis))
> A::A ()
> {
>   ptr = mmap ((void *) 0x3600000000UL, 65536, PROT_READ|PROT_WRITE,
>               MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> }
>
> A __attribute__((init_priority (1))) a;
>
> int
> main ()
> {
>   if (a.ptr != MAP_FAILED)
>     {
>       char *ptr = (char *) a.ptr;
>       __asan_poison_memory_region (ptr, 4096);
>       __asan_poison_memory_region (ptr + 61440, 4096);
>       ptr[4096] = 23;
>     }
> }
>
> and similar (and/or test that accessing the poisoned memory fails etc.).
> For gcc you want to compile with -w, so that it doesn't warn about the
> reserved init_priority, not sure what clang would do.

This is ungood.
First, clang doesn't like it at all:
prelink1.cc:18:18: error: init_priority attribute requires integer
constant between 101 and 65535 inclusive
A __attribute__((init_priority (1))) a;

Second, in some settings we are using ASAN_USE_PREINIT_ARRAY=1
and this may become the default on linux at some point.
so, __asan_init will get called before A::A()

Waiting for Ian's reply about gold.

--kcc

>
>         Jakub



More information about the Gcc-patches mailing list