[PATCH] D24131: Add NewAddressDescription, which can describe any type of address.

[Filipe Cabecinhas via llvm-commits]

But then you end up needlessly locking the registry. We only need to
lock it when querying the stacks.

Since you can call it from user code, I don't think we should be doing
that, as we'd be making the critical section much bigger than it
should be.
Since the other patches depend on this one, you don't see it being
used yet, but I've already posted several patches that need it:
D24389, D24390, D24391, D24392, D24393. (Those patches aren't yet
based on this newer version of this patch. I will updated them soon).

Thank you,
 Filipe


On Fri, Sep 9, 2016 at 9:36 PM, Vitaly Buka <vitalybuka at google.com> wrote:
> I see. This is not on the patch again.
> This arg is not obvious, called needs to make decision anyway.
>
> Maybe just remove the arg, and make caller to lock the registry:
>
> AddressDescription(uptr addr, uptr length) {
>   asanThreadRegistry().CheckLocked();  // as first line, to fail soon.
>   ...
> }
>
>
> On Fri, Sep 9, 2016 at 11:43 AM Filipe Cabecinhas
> <filcab+llvm.phabricator at gmail.com> wrote:
>>
>> __asan_describe_address always needs the lock
>> But the Error* descriptions have the lock already and you can't double
>> lock.
>>
>>  Filipe
>>
>>
>> On Friday, 9 September 2016, Vitaly Buka <vitalybuka at google.com> wrote:
>>>
>>> vitalybuka added inline comments.
>>>
>>> ================
>>> Comment at: lib/asan/asan_descriptions.h:197
>>> @@ +196,3 @@
>>> +  AddressDescription(uptr addr, uptr length,
>>> +                     bool shouldLockThreadRegistry = true) {
>>> +    if (GetShadowAddressInformation(addr, &data.shadow)) {
>>> ----------------
>>> And I'd prefer for now remove shouldLockThreadRegistry argument and do
>>> ThreadRegistryLock inside of GetStackAddressInformation uconditionally
>>>
>>>
>>> https://reviews.llvm.org/D24131
>>>
>>>
>>>
>