[Lldb-commits] [lldb] r169645 - in /lldb/trunk: include/lldb/Core/ include/lldb/Expression/ source/Commands/ source/Core/ source/Expression/ source/Interpreter/ source/Plugins/DynamicLoader/MacOSX-DYLD/ source/Plugins/LanguageRuntime/ObjC/AppleObjC...

[Filipe Cabecinhas]

Sorry for not replying before it was commited, but yes, seems okay.

Regards,

  Filipe

  F



On Thu, Dec 13, 2012 at 11:54 AM, Kopec, Matt <matt.kopec at intel.com> wrote:

> Hi Filipe,
>
> Thanks for the heads up. After some further review, we removed the memset
> altogether. We copy the bytes from dst to an unsigned int and send that for
> logging instead so the memset isn't needed anymore. This will ensure only
> the buffer data will be output and in the same format as before. Patch
> attached.
>
> Thanks,
> Matt
> ________________________________________
> From: lldb-commits-bounces at cs.uiuc.edu [lldb-commits-bounces at cs.uiuc.edu]
> on behalf of Malea, Daniel [daniel.malea at intel.com]
> Sent: Wednesday, December 12, 2012 8:42 PM
> To: Filipe Cabecinhas
> Cc: lldb-commits
> Subject: Re: [Lldb-commits] [lldb] r169645 - in /lldb/trunk:
> include/lldb/Core/ include/lldb/Expression/ source/Commands/ source/Core/
> source/Expression/ source/Interpreter/
> source/Plugins/DynamicLoader/MacOSX-DYLD/
> source/Plugins/LanguageRuntime/ObjC/AppleObjC...
>
> Hi Filipe,
>
> Thanks for catching this! Matt and I discussed the Printf() call and we're
> pretty sure it can lead to some problems...
>
> On 2012-12-12, at 6:41 PM, Filipe Cabecinhas wrote:
>
> I'm not familiar with the code on this part, but I would say that we want
> one of these:
>   - change the memset size to sizeof(unsigned long*)
>   - change the memset size to size, which I suppose is the size we have in
> the memory pointed to by buf (which gets static_cast'ed to dst). Are we
> always sure size is equal or greater than sizeof(unsigned long)?
>
> Or maybe I didn't understand this part of the code.
> Could someone more familiar with ProcessMonitor chip in?
>
>
> We will change the memset length to "size", but also (because dst gets
> incremented as part of the for loop on line 241) we will move the memset()
> call before the loop.
>
> You brought up a very good point though about size being equal or greater
> than sizeof(unsigned long)... I'm not sure there's any guarantees that a
> caller of DoReadMemory doesn't want to read a single byte by passing in a
> buffer of size 1. In case logging is enabled, this might lead to some
> weirdness. We have a fix, but it introduces some more bit-shifting magic.
> Matt or I will post it on the list tomorrow if we don't come up with
> something more elegant.
>
>
>
> Daniel Malea <daniel.malea at intel.com<mailto:daniel.malea at intel.com>>
> Intel Waterloo
> Phone: 519-772-2566
>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20121214/31fbfef6/attachment.html>