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

[Kopec, Matt]

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 --------------
A non-text attachment was scrubbed...
Name: loggingfix.patch
Type: text/x-patch
Size: 1492 bytes
Desc: loggingfix.patch
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20121213/fce33189/attachment.bin>