[Lldb-commits] [PATCH] Don't use a random probe / allocation scheme in the IRMemoryMap

[Zachary Turner]

Hope it's ok to bump this again.  No worries if you're busy, just want to
make sure this doesn't get lost.


On Thu, Jun 26, 2014 at 7:49 PM, Zachary Turner <zturner at google.com> wrote:

> +scallanan
>
> I tried to add you to the review through Phab, but it looks like you
> weren't registered.  Thanks
>
>
> On Wed, Jun 25, 2014 at 4:15 PM, Zachary Turner <zturner at google.com>
> wrote:
>
>> The current strategy for host allocation is to choose a random address
>> and attempt to allocate there, eventually failing if the allocation cannot
>> be satisfied.
>>
>> The C standard only guarantees that RAND_MAX >= 32767, so for platforms
>> where this is true allocations will fail with very high probability.   On
>> such platforms, you can reproduce this trivially by running lldb, typing
>> "expr (3)" and then hitting enter you see a failure.  Failures generally
>> happen with a frequency of about 1 failure every 5 evaluations.
>>
>> I cannot come up with a good reason that the allocations need to look
>> like "real" pointers, so this patch changes the allocation scheme to simply
>> jump straight to the end and grab a free chunk of memory.
>>
>> http://reviews.llvm.org/D4300
>>
>> Files:
>>   source/Expression/IRMemoryMap.cpp
>>
>> Index: source/Expression/IRMemoryMap.cpp
>> ===================================================================
>> --- source/Expression/IRMemoryMap.cpp
>> +++ source/Expression/IRMemoryMap.cpp
>> @@ -53,6 +53,8 @@
>>      lldb::ProcessSP process_sp = m_process_wp.lock();
>>
>>      lldb::addr_t ret = LLDB_INVALID_ADDRESS;
>> +    if (size == 0)
>> +      return ret;
>>
>>      if (process_sp && process_sp->CanJIT() && process_sp->IsAlive())
>>      {
>> @@ -66,37 +68,13 @@
>>              return ret;
>>      }
>>
>> -    for (int iterations = 0; iterations < 16; ++iterations)
>> -    {
>> -        lldb::addr_t candidate = LLDB_INVALID_ADDRESS;
>> -
>> -        switch (target_sp->GetArchitecture().GetAddressByteSize())
>> -        {
>> -        case 4:
>> -            {
>> -                uint32_t random_data = rand();
>> -                candidate = random_data;
>> -                candidate &= ~0xfffull;
>> -                break;
>> -            }
>> -        case 8:
>> -            {
>> -                uint32_t random_low = rand();
>> -                uint32_t random_high = rand();
>> -                candidate = random_high;
>> -                candidate <<= 32ull;
>> -                candidate |= random_low;
>> -                candidate &= ~0xfffull;
>> -                break;
>> -            }
>> -        }
>> -
>> -        if (IntersectsAllocation(candidate, size))
>> -            continue;
>> -
>> -        ret = candidate;
>> -
>> -        return ret;
>> +    // Don't allocate from the zero page.
>> +    ret = 0x1000;
>> +    if (!m_allocations.empty()) {
>> +      auto back = m_allocations.rbegin();
>> +      lldb::addr_t addr = back->first;
>> +      size_t size = back->second.m_size;
>> +      ret = llvm::RoundUpToAlignment(addr+size, 4096);
>>      }
>>
>>      return ret;
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140630/2472a45a/attachment.html>