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

[Alexey Samsonov]

On Fri, Aug 31, 2012 at 9:22 PM, David Blaikie <dblaikie at gmail.com> wrote:

> 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).
>

I think you're right and it's cleaner to remove implicit conversion here
(submitted).

>
> 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.
>

Thanks!


> - 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
> >
>



-- 
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120905/b43be48c/attachment.html>