{"id":704,"date":"2006-05-30T15:35:54","date_gmt":"2006-05-30T05:35:54","guid":{"rendered":"http:\/\/www.flamingspork.com\/blog\/2006\/05\/30\/i-heart-valgrind-or-an-early-patch-integrating-the-mysql-mem_root-stuff-with-valgrind\/"},"modified":"2006-05-30T15:39:33","modified_gmt":"2006-05-30T05:39:33","slug":"i-heart-valgrind-or-an-early-patch-integrating-the-mysql-mem_root-stuff-with-valgrind","status":"publish","type":"post","link":"https:\/\/www.flamingspork.com\/blog\/2006\/05\/30\/i-heart-valgrind-or-an-early-patch-integrating-the-mysql-mem_root-stuff-with-valgrind\/","title":{"rendered":"I heart valgrind (or: an early patch integrating the MySQL MEM_ROOT stuff with valgrind)"},"content":{"rendered":"<p>Everybody knows that <a href=\"http:\/\/www.valgrind.org\">valgrind<\/a> is great.<\/p>\n<p>Well, I was observing a problem in some MySQL code, it looked like we were writing over some memory that we weren&#8217;t meant to be (as the structure hadn&#8217;t been initialised yet). But, seeing as this was memory that had been allocated off a MEM_ROOT (one of our memory allocators), valgrind wasn&#8217;t gonig to spit out anything.<\/p>\n<p>This is because this bit of memory had already been allocated and subsequently &#8220;freed&#8221;, but then reallocated. The &#8220;free&#8221;ing overwrites the memory with garbage (which is what the MEM_ROOT code does) so that you should see crashes (and a pattern) when you do something bad.<\/p>\n<p>The traditional way to troubleshoot this in to modify your memory allocator so that it just calls malloc() and free() all the time (and valgrind will trap them). We have some code in there to do that too. However, this requires changing some ifdefs and probably not being as efficient.<\/p>\n<p>Valgrind has some macros you can insert into your memory allocator code that tell valgrind that you have &#8220;freed&#8221; this memory (VALGRIND_MAKE_NOACCESS) or have allocated it (VALGRIND_MAKE_WRITABLE) or have written valid data to it (VALGRIND_MAKE_READABLE). These are semi-documented in the valgrind\/valgrind.h file.<\/p>\n<p>These are designed to only add a few CPU instructions to your code, so it should be possible to always have them in your build (you can disable them donig anything by building with -DNVALGRIND IIRC).<\/p>\n<p>(I haven&#8217;t done any benchmarks on the code to see if there is any impact though).<\/p>\n<p>Valgrind also has a great (largely undocumented) feature of being able to integrate with memory pools. Since our MEM_ROOT is largely just this, we can get some added benefits here too (one should be better valgrind warnings when we do some bad stuff).<br \/>\nIt lets you associate memory with a memory pool, and then just say &#8220;this pool has been freed&#8221;. Saves you having to keep track of each pointer in the code to pass to &#8220;free&#8221;. It also can give you valgrind warnings when you try and allocate memory to something that hasn&#8217;t been initialised as a memory pool.<\/p>\n<p>The most interesting thing of writing the patch was finding some false positive warnings. Namely, a trick used in a couple of places (i see 2) in the code is to create a temporary memory root on the stack, allocate a larger block of memory and then &#8220;swap&#8221; the memory roots to be based in this block of memory. I had to write a swap_root function to implement this as valgrind doesn&#8217;t export a &#8220;swap memory pool&#8221; function. It would be a useful addition, maybe I&#8217;ll go and suggest it to the developers.<\/p>\n<p>Anyway, I got over that hurdle and now have this patch which seems to work pretty well. I still get a couple of (possible) false positives. We&#8217;ll see if this finds any neat bugs. Also, a good exercise would be to see how many extra instructions are really generated and if this has any affect on performance at all.<\/p>\n<p><code>    ===== include\/my_sys.h 1.196 vs edited =====<br \/>\n--- 1.196\/include\/my_sys.h\t2006-05-22 20:04:34 +10:00<br \/>\n+++ edited\/include\/my_sys.h\t2006-05-26 16:22:11 +10:00<br \/>\n@@ -804,6 +804,7 @@<br \/>\nextern void set_prealloc_root(MEM_ROOT *root, char *ptr);<br \/>\nextern void reset_root_defaults(MEM_ROOT *mem_root, uint block_size,<br \/>\nuint prealloc_size);<br \/>\n+extern void swap_root(MEM_ROOT* new_root, MEM_ROOT* old);<br \/>\nextern char *strdup_root(MEM_ROOT *root,const char *str);<br \/>\nextern char *strmake_root(MEM_ROOT *root,const char *str,uint len);<br \/>\nextern char *memdup_root(MEM_ROOT *root,const char *str,uint len);<br \/>\n===== mysys\/my_alloc.c 1.33 vs edited =====<br \/>\n--- 1.33\/mysys\/my_alloc.c\t2005-11-24 07:44:54 +11:00<br \/>\n+++ edited\/mysys\/my_alloc.c\t2006-05-26 19:21:12 +10:00<br \/>\n@@ -22,6 +22,8 @@<br \/>\n#undef EXTRA_DEBUG<br \/>\n#define EXTRA_DEBUG<br \/>\n+#include \"valgrind\/valgrind.h\"<br \/>\n+#include \"valgrind\/memcheck.h\"<\/code><\/p>\n<p>\/*<br \/>\nInitialize memory root<br \/>\n@@ -66,9 +68,12 @@<br \/>\nmem_root->free->size= pre_alloc_size+ALIGN_SIZE(sizeof(USED_MEM));<br \/>\nmem_root->free->left= pre_alloc_size;<br \/>\nmem_root->free->next= 0;<br \/>\n+      VALGRIND_MAKE_NOACCESS(mem_root->free+ALIGN_SIZE(sizeof(USED_MEM)),<br \/>\n+                             pre_alloc_size);<br \/>\n}<br \/>\n}<br \/>\n#endif<br \/>\n+  VALGRIND_CREATE_MEMPOOL(mem_root,0,0);<br \/>\nDBUG_VOID_RETURN;<br \/>\n}<\/p>\n<p>@@ -217,6 +222,9 @@<br \/>\nmem_root->first_block_usage= 0;<br \/>\n}<br \/>\nDBUG_PRINT(&#8220;exit&#8221;,(&#8220;ptr: 0x%lx&#8221;, (ulong) point));<br \/>\n+\/\/  fprintf(stderr,&#8221;root: %lx point: %lx size:%lx\\n&#8221;,mem_root,point,Size);<br \/>\n+  VALGRIND_MEMPOOL_ALLOC(mem_root,point,Size);<br \/>\n+  VALGRIND_MAKE_WRITABLE(point,Size);<br \/>\nDBUG_RETURN(point);<br \/>\n#endif<br \/>\n}<br \/>\n@@ -286,7 +294,8 @@<br \/>\nfor (next= root->free; next; next= *(last= &#038;next->next))<br \/>\n{<br \/>\nnext->left= next->size &#8211; ALIGN_SIZE(sizeof(USED_MEM));<br \/>\n&#8211;    TRASH_MEM(next);<br \/>\n+    VALGRIND_MAKE_NOACCESS(next+ALIGN_SIZE(sizeof(USED_MEM)),next->left);<br \/>\n+\/\/    TRASH_MEM(next);<br \/>\n}<\/p>\n<p>\/* Combine the free and the used list *\/<br \/>\n@@ -296,7 +305,8 @@<br \/>\nfor (; next; next= next->next)<br \/>\n{<br \/>\nnext->left= next->size &#8211; ALIGN_SIZE(sizeof(USED_MEM));<br \/>\n&#8211;    TRASH_MEM(next);<br \/>\n+    VALGRIND_MAKE_NOACCESS(next+ALIGN_SIZE(sizeof(USED_MEM)),next->left);<br \/>\n+\/\/    TRASH_MEM(next);<br \/>\n}<\/p>\n<p>\/* Now everything is set; Indicate that nothing is used anymore *\/<br \/>\n@@ -357,12 +367,55 @@<br \/>\n{<br \/>\nroot->free=root->pre_alloc;<br \/>\nroot->free->left=root->pre_alloc->size-ALIGN_SIZE(sizeof(USED_MEM));<br \/>\n&#8211;    TRASH_MEM(root->pre_alloc);<br \/>\n+    \/\/TRASH_MEM(root->pre_alloc);<br \/>\nroot->free->next=0;<br \/>\n}<br \/>\nroot->block_num= 4;<br \/>\nroot->first_block_usage= 0;<br \/>\n+  VALGRIND_DESTROY_MEMPOOL(root);<br \/>\n+  VALGRIND_CREATE_MEMPOOL(root,0,0);<br \/>\n+  VALGRIND_MAKE_READABLE(root,sizeof(MEM_ROOT));<br \/>\n+  if(root->pre_alloc)<br \/>\n+  {<br \/>\n+    VALGRIND_MAKE_READABLE(root->pre_alloc, ALIGN_SIZE(sizeof(USED_MEM)));<br \/>\n+    VALGRIND_MEMPOOL_ALLOC(root,root->pre_alloc,root->pre_alloc->size);<br \/>\n+    VALGRIND_MAKE_READABLE(root->pre_alloc, ALIGN_SIZE(sizeof(USED_MEM)));<br \/>\n+  }<br \/>\nDBUG_VOID_RETURN;<br \/>\n+}<br \/>\n+<br \/>\n+void swap_root(MEM_ROOT* new_root, MEM_ROOT* old)<br \/>\n+{<br \/>\n+  memcpy((char*) new_root, (char*) old, sizeof(MEM_ROOT));<br \/>\n+  VALGRIND_DESTROY_MEMPOOL(old);<br \/>\n+  VALGRIND_CREATE_MEMPOOL(new_root,0,0);<br \/>\n+<br \/>\n+  reg1 USED_MEM *next;<br \/>\n+<br \/>\n+  VALGRIND_MEMPOOL_ALLOC(new_root,new_root,sizeof(MEM_ROOT));<br \/>\n+  VALGRIND_MAKE_READABLE(new_root,sizeof(MEM_ROOT));<br \/>\n+<br \/>\n+  \/* iterate through (partially) free blocks *\/<br \/>\n+  next= new_root->free;<br \/>\n+  do<br \/>\n+  {<br \/>\n+    if(!next)<br \/>\n+      break;<br \/>\n+    VALGRIND_MEMPOOL_ALLOC(new_root,next,next->size-next->left);<br \/>\n+    VALGRIND_MAKE_READABLE(next,next->size-next->left);<br \/>\n+    next= next->next;<br \/>\n+  } while(1);<br \/>\n+<br \/>\n+  \/* now go through the used blocks and mark them free *\/<br \/>\n+  next= new_root->used;<br \/>\n+  do<br \/>\n+  {<br \/>\n+    if(!next)<br \/>\n+      break;<br \/>\n+    VALGRIND_MEMPOOL_ALLOC(new_root,next,next->size-next->left);<br \/>\n+    VALGRIND_MAKE_READABLE(next,next->size-next->left);<br \/>\n+    next= next->next;<br \/>\n+  } while(1);<br \/>\n}<\/p>\n<p>\/*<br \/>\n===== sql\/table.cc 1.215 vs edited =====<br \/>\n&#8212; 1.215\/sql\/table.cc\t2006-05-23 05:54:55 +10:00<br \/>\n+++ edited\/sql\/table.cc\t2006-05-26 18:12:21 +10:00<br \/>\n@@ -150,7 +150,8 @@<\/p>\n<p>#endif<\/p>\n<p>&#8211;    memcpy((char*) &#038;share->mem_root, (char*) &#038;mem_root, sizeof(mem_root));<br \/>\n+\/\/    memcpy((char*) &#038;share->mem_root, (char*) &#038;mem_root, sizeof(mem_root));<br \/>\n+    swap_root(&#038;share->mem_root,&#038;mem_root);<br \/>\npthread_mutex_init(&#038;share->mutex, MY_MUTEX_INIT_FAST);<br \/>\npthread_cond_init(&#038;share->cond, NULL);<br \/>\n}<br \/>\n@@ -252,7 +253,7 @@<br \/>\nhash_free(&#038;share->name_hash);<\/p>\n<p>\/* We must copy mem_root from share because share is allocated through it *\/<br \/>\n&#8211;  memcpy((char*) &#038;mem_root, (char*) &#038;share->mem_root, sizeof(mem_root));<br \/>\n+ swap_root(&#038;mem_root,&#038;share->mem_root);\/\/memcpy((char*) &#038;mem_root, (char*) &#038;share->mem_root, sizeof(mem_root));<br \/>\nfree_root(&#038;mem_root, MYF(0));                 \/\/ Free&#8217;s share<br \/>\nDBUG_VOID_RETURN;<br \/>\n}<br \/>\n===== storage\/ndb\/src\/kernel\/blocks\/dbdict\/Dbdict.cpp 1.87 vs edited =====<br \/>\n&#8212; 1.87\/storage\/ndb\/src\/kernel\/blocks\/dbdict\/Dbdict.cpp\t2006-04-25 22:02:07 +10:00<br \/>\n+++ edited\/storage\/ndb\/src\/kernel\/blocks\/dbdict\/Dbdict.cpp\t2006-05-26 12:15:43 +10:00<br \/>\n@@ -3148,9 +3148,23 @@<\/p>\n<p>CreateTableRecordPtr createTabPtr;<br \/>\nndbrequire(c_opCreateTable.find(createTabPtr, callbackData));<br \/>\n&#8211;<br \/>\n&#8211;  \/\/@todo check error<br \/>\n&#8211;  ndbrequire(createTabPtr.p->m_errorCode == 0);<br \/>\n+<br \/>\n+  if(createTabPtr.p->m_errorCode != 0)<br \/>\n+  {<br \/>\n+    char buf[255];<br \/>\n+    TableRecordPtr tabPtr;<br \/>\n+    c_tableRecordPool.getPtr(tabPtr, createTabPtr.p->m_tablePtrI);<br \/>\n+<br \/>\n+    BaseString::snprintf(buf, sizeof(buf),<br \/>\n+\t   &#8220;Unable to restart, fail while creating table %d&#8221;<br \/>\n+\t   &#8221; error: %d. Most likely change of configuration&#8221;,<br \/>\n+\t   tabPtr.p->tableId,<br \/>\n+\t   createTabPtr.p->m_errorCode);<br \/>\n+    progError(__LINE__,<br \/>\n+\t      NDBD_EXIT_INVALID_CONFIG,<br \/>\n+\t      buf);<br \/>\n+    ndbrequire(createTabPtr.p->m_errorCode == 0);<br \/>\n+  }<\/p>\n<p>Callback callback;<br \/>\ncallback.m_callbackData = callbackData;<\/p>\n","protected":false},"excerpt":{"rendered":"<p>Everybody knows that valgrind is great. Well, I was observing a problem in some MySQL code, it looked like we were writing over some memory that we weren&#8217;t meant to be (as the structure hadn&#8217;t been initialised yet). But, seeing &hellip; <a href=\"https:\/\/www.flamingspork.com\/blog\/2006\/05\/30\/i-heart-valgrind-or-an-early-patch-integrating-the-mysql-mem_root-stuff-with-valgrind\/\">Continue reading <span class=\"meta-nav\">&rarr;<\/span><\/a><\/p>\n","protected":false},"author":2,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"_jetpack_newsletter_access":"","_jetpack_dont_email_post_to_subs":false,"_jetpack_newsletter_tier_id":0,"_jetpack_memberships_contains_paywalled_content":false,"_jetpack_memberships_contains_paid_content":false,"footnotes":"","jetpack_publicize_message":"","jetpack_publicize_feature_enabled":true,"jetpack_social_post_already_shared":false,"jetpack_social_options":{"image_generator_settings":{"template":"highway","default_image_id":0,"font":"","enabled":false},"version":2},"jetpack_post_was_ever_published":false},"categories":[14],"tags":[],"class_list":["post-704","post","type-post","status-publish","format-standard","hentry","category-mysql"],"jetpack_publicize_connections":[],"jetpack_featured_media_url":"","jetpack_sharing_enabled":true,"jetpack_shortlink":"https:\/\/wp.me\/p5a6n8-bm","jetpack-related-posts":[{"id":415,"url":"https:\/\/www.flamingspork.com\/blog\/2005\/05\/27\/helgrind\/","url_meta":{"origin":704,"position":0},"title":"Helgrind","author":"Stewart Smith","date":"2005-05-27","format":false,"excerpt":"To try and help in debugging, I've been playing with some of the extra tools that come with Valgrind. Everybody knows that if you aren't using Valgrind you are living in sin. I've recently tried to have a go at using Helgrind. It's suppossed to be able to help you\u2026","rel":"","context":"In &quot;General&quot;","block_context":{"text":"General","link":"https:\/\/www.flamingspork.com\/blog\/category\/general\/"},"img":{"alt_text":"","src":"","width":0,"height":0},"classes":[]},{"id":4035,"url":"https:\/\/www.flamingspork.com\/blog\/2016\/02\/26\/mysql-contributions-status\/","url_meta":{"origin":704,"position":1},"title":"MySQL Contributions status","author":"Stewart Smith","date":"2016-02-26","format":false,"excerpt":"This post is an update to the status of various MySQL bugs (some with patches) that I've filed over the past couple of years (or that people around me have). I'm not looking at POWER specific ones, as there are these too, but each of these bugs here deal with\u2026","rel":"","context":"In &quot;code&quot;","block_context":{"text":"code","link":"https:\/\/www.flamingspork.com\/blog\/category\/code\/"},"img":{"alt_text":"","src":"","width":0,"height":0},"classes":[]},{"id":638,"url":"https:\/\/www.flamingspork.com\/blog\/2006\/04\/05\/finding-the-cause-of-a-bug-lesson-1\/","url_meta":{"origin":704,"position":2},"title":"Finding the cause of a bug Lesson 1","author":"Stewart Smith","date":"2006-04-05","format":false,"excerpt":"Assume nothing. Your assumptions are wrong, that's why there's a bug silly! If valgrind had a time machine function it'd be totally awesome. But it doesn't so currently extra work and thinking is required. doh!","rel":"","context":"In &quot;mysql&quot;","block_context":{"text":"mysql","link":"https:\/\/www.flamingspork.com\/blog\/category\/work-et-al\/mysql\/"},"img":{"alt_text":"","src":"","width":0,"height":0},"classes":[]},{"id":1110,"url":"https:\/\/www.flamingspork.com\/blog\/2008\/05\/21\/twitter-updates-for-2008-05-21\/","url_meta":{"origin":704,"position":3},"title":"Twitter Updates for 2008-05-21","author":"Stewart Smith","date":"2008-05-21","format":false,"excerpt":"lets get me my twitter on # @dormando: valgrind! # Powered by Twitter Tools.","rel":"","context":"In &quot;Twitter&quot;","block_context":{"text":"Twitter","link":"https:\/\/www.flamingspork.com\/blog\/category\/from-twitter\/"},"img":{"alt_text":"","src":"","width":0,"height":0},"classes":[]},{"id":4057,"url":"https:\/\/www.flamingspork.com\/blog\/2016\/05\/19\/fuzzing-firmware-afl-fuzz-skiboot\/","url_meta":{"origin":704,"position":4},"title":"Fuzzing Firmware &#8211; afl-fuzz + skiboot","author":"Stewart Smith","date":"2016-05-19","format":false,"excerpt":"In what is likely to be a series on how firmware makes some normal tools harder to use, first I'm going to look at american fuzzy lop - a tool for fuzz testing that if you're not using then you most certainly have bugs it'll find for you. I first\u2026","rel":"","context":"In &quot;code&quot;","block_context":{"text":"code","link":"https:\/\/www.flamingspork.com\/blog\/category\/code\/"},"img":{"alt_text":"","src":"","width":0,"height":0},"classes":[]},{"id":1780,"url":"https:\/\/www.flamingspork.com\/blog\/2010\/01\/18\/you-have-already-lost\/","url_meta":{"origin":704,"position":5},"title":"You have already lost","author":"Stewart Smith","date":"2010-01-18","format":false,"excerpt":"When the following code introduces a valgrind warning... you are in a world of pain and loss: === modified file 'drizzled\/field\/blob.h' --- drizzled\/field\/blob.h 2009-12-21 08:16:13 +0000 +++ drizzled\/field\/blob.h 2010-01-18 01:36:48 +0000 @@ -32,6 +32,7 @@ *\/ class Field_blob :public Field_str { protected: + uint32_t assassass; uint32_t packlength; String value; \/\/\u2026","rel":"","context":"In &quot;General&quot;","block_context":{"text":"General","link":"https:\/\/www.flamingspork.com\/blog\/category\/general\/"},"img":{"alt_text":"","src":"","width":0,"height":0},"classes":[]}],"jetpack_likes_enabled":true,"_links":{"self":[{"href":"https:\/\/www.flamingspork.com\/blog\/wp-json\/wp\/v2\/posts\/704","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/www.flamingspork.com\/blog\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/www.flamingspork.com\/blog\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/www.flamingspork.com\/blog\/wp-json\/wp\/v2\/users\/2"}],"replies":[{"embeddable":true,"href":"https:\/\/www.flamingspork.com\/blog\/wp-json\/wp\/v2\/comments?post=704"}],"version-history":[{"count":0,"href":"https:\/\/www.flamingspork.com\/blog\/wp-json\/wp\/v2\/posts\/704\/revisions"}],"wp:attachment":[{"href":"https:\/\/www.flamingspork.com\/blog\/wp-json\/wp\/v2\/media?parent=704"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/www.flamingspork.com\/blog\/wp-json\/wp\/v2\/categories?post=704"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/www.flamingspork.com\/blog\/wp-json\/wp\/v2\/tags?post=704"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}