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

[Filipe Cabecinhas via llvm-commits]

Sorry about the delay in filing the PR. I will file one and add
references to it in the commit messages.

Thank you,
 Filipe

On Fri, Sep 9, 2016 at 10:33 PM, Vitaly Buka <vitalybuka at google.com> wrote:
> Not big difference, but in general i'd prefer this committed with
> unconditional locking, and with no argument. And argument is being added in
> the patch where you use if from coded where it set to false.
>
> So lets keep it as is and reconsider locking later.
>
> And looks like you don't want attach PR? Maybe it's too late.
>
>
>
> On Fri, Sep 9, 2016 at 2:05 PM Filipe Cabecinhas
> <filcab+llvm.phabricator at gmail.com> wrote:
>>
>> 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
>> >>>
>> >>>
>> >>>
>> >