[llvm-commits] [compiler-rt] r162990 - in /compiler-rt/trunk/lib/sanitizer_common: sanitizer_symbolizer.cc sanitizer_symbolizer_linux.cc

[David Blaikie]

On Fri, Aug 31, 2012 at 10:12 AM, Alexey Samsonov <samsonov at google.com> wrote:
>
>
> On Fri, Aug 31, 2012 at 9:06 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> On Fri, Aug 31, 2012 at 4:43 AM, Alexey Samsonov <samsonov at google.com>
>> wrote:
>> > Author: samsonov
>> > Date: Fri Aug 31 06:43:01 2012
>> > New Revision: 162990
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=162990&view=rev
>> > Log:
>> > [Sanitizer] fix overloaded operator error
>> >
>> > Modified:
>> >     compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc
>> >     compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_linux.cc
>> >
>> > Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc?rev=162990&r1=162989&r2=162990&view=diff
>> >
>> > ==============================================================================
>> > --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc
>> > (original)
>> > +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc Fri
>> > Aug 31 06:43:01 2012
>> > @@ -255,7 +255,7 @@
>> >    }
>> >
>> >    static const uptr kMaxNumberOfModuleContexts = 4096;
>> > -  LoadedModule *modules_; // Array of module descriptions is leaked.
>> > +  LoadedModule *modules_;  // Array of module descriptions is leaked.
>> >    uptr n_modules_;
>> >
>> >    ExternalSymbolizer *external_symbolizer_;  // Leaked.
>> >
>> > Modified:
>> > compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_linux.cc
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_linux.cc?rev=162990&r1=162989&r2=162990&view=diff
>> >
>> > ==============================================================================
>> > --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_linux.cc
>> > (original)
>> > +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_linux.cc
>> > Fri Aug 31 06:43:01 2012
>> > @@ -122,7 +122,7 @@
>> >    if (data->current_n == data->max_n)
>> >      return 0;
>> >    InternalScopedBuffer<char> module_name(kMaxPathLength);
>> > -  module_name[0] = '\0';
>> > +  module_name.data()[0] = '\0';
>>
>> What's the purpose of this change?
>>
>> From what I can see, these two constructs look equivalent, given that
>> the op[] and data() members are defined as:
>>
>>   T &operator[](uptr i) { return ptr_[i]; }
>>   T *data() { return ptr_; }
>
>
> ../sanitizer_common/sanitizer_symbolizer_linux.cc:125:14: error: use of
> overloaded operator '[]' is ambiguous (with operand types
> 'InternalScopedBuffer<char>' and 'int')
>   module_name[0] = '\0';
>   ~~~~~~~~~~~^~
> ../sanitizer_common/sanitizer_common.h:71:6: note: candidate function
>   T &operator[](uptr i) { return ptr_[i]; }
>      ^
> ../sanitizer_common/sanitizer_symbolizer_linux.cc:125:14: note: built-in
> candidate operator[](char *, int)
>   module_name[0] = '\0';

Ah, so the implicit conversion to T* is causing this? (is that
implicit conversion really the best idea? the "data()" member function
seems like it might be the better option - implicit conversions can
get a bit surprising, as is the case here).

Alternatively you could use 0ul, probably. Though both versions seem a
bit subtle when a reader comes back & wonders why it was written that
way.

Anyway, just a thought.
- David

>>
>>
>> >    if (data->current_n == 0) {
>> >      // First module is the binary itself.
>> >      uptr module_name_len = readlink("/proc/self/exe",
>> > @@ -133,7 +133,7 @@
>> >    } else if (info->dlpi_name) {
>> >      internal_strncpy(module_name, info->dlpi_name, module_name.size());
>> >    }
>> > -  if (module_name[0] == '\0')
>> > +  if (module_name.data()[0] == '\0')
>> >      return 0;
>> >    void *mem = &data->modules[data->current_n];
>> >    LoadedModule *cur_module = new(mem) LoadedModule(module_name,
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
>
> --
> Alexey Samsonov, MSK
>