{"thread":{"id":"41178","subject":"[PATCH] Makefile: describe XMALLOC_POISON","startedAt":"2016-01-13T11:57:35Z","lastAt":"2016-01-13T18:23:09Z","messageCount":3,"participants":["Alexander Kuleshov","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"275890","messageId":"1452686255-8757-1-git-send-email-kuleshovmail@gmail.com","threadId":"41178","inReplyTo":null,"subject":"[PATCH] Makefile: describe XMALLOC_POISON","fromName":"Alexander Kuleshov","fromEmail":"kuleshovmail@gmail.com","sentAt":"2016-01-13T11:57:35Z","receivedAt":"2016-01-13T11:57:35Z","isPatch":true,"sender":{"key":"kuleshovmail@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2699235?v=4"},"body":"The do_xmalloc() functions may fill an allocated buffer with the\nknown value (0xA5) for debugging if we will pass the XMALLOC_POISON\noption during build.\n\nThis patch adds description of this option to the Makefile.\n\nSigned-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>\n---\n Makefile | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex f3325de..673c244 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -367,6 +367,10 @@ all::\n # Define HAVE_BSD_SYSCTL if your platform has a BSD-compatible sysctl function.\n #\n # Define HAVE_GETDELIM if your system has the getdelim() function.\n+#\n+# Define XMALLOC_POISON if you are debugging the xmalloc(). In a XMALLOC_POISON\n+# build, each allocated buffer by the xmalloc() will be in known state. After\n+# memory allocation, a buffer will be filled with '0xA5' values.\n \n GIT-VERSION-FILE: FORCE\n \t@$(SHELL_PATH) ./GIT-VERSION-GEN\n-- \n2.7.0.25.gfc10eb5.dirty\n"},{"id":"275924","messageId":"1452704204-1928-1-git-send-email-kuleshovmail@gmail.com","threadId":"41178","inReplyTo":"1452686255-8757-1-git-send-email-kuleshovmail@gmail.com","subject":"[PATCH] Makefile: describe XMALLOC_POISON","fromName":"Alexander Kuleshov","fromEmail":"kuleshovmail@gmail.com","sentAt":"2016-01-13T16:56:44Z","receivedAt":"2016-01-13T16:56:44Z","isPatch":true,"sender":{"key":"kuleshovmail@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2699235?v=4"},"body":"The do_xmalloc() functions may fill an allocated buffer with the\nknown value (0xA5) for debugging if we will pass the XMALLOC_POISON\noption during build.\n\nThis patch adds description of this option to the Makefile and\nadds it to BASIC_CFLAGS if it was provided.\n\nSigned-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>\n---\n Makefile | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex f3325de..3f942b5 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -367,6 +367,10 @@ all::\n # Define HAVE_BSD_SYSCTL if your platform has a BSD-compatible sysctl function.\n #\n # Define HAVE_GETDELIM if your system has the getdelim() function.\n+#\n+# Define XMALLOC_POISON if you are debugging the xmalloc(). In a XMALLOC_POISON\n+# build, each allocated buffer by the xmalloc() will be in known state. After\n+# memory allocation, a buffer will be filled with '0xA5' values.\n \n GIT-VERSION-FILE: FORCE\n \t@$(SHELL_PATH) ./GIT-VERSION-GEN\n@@ -1481,6 +1485,10 @@ ifdef HAVE_GETDELIM\n \tBASIC_CFLAGS += -DHAVE_GETDELIM\n endif\n \n+ifdef XMALLOC_POISON\n+\tBASIC_CFLAGS += -DXMALLOC_POISON\n+endif\n+\n ifeq ($(TCLTK_PATH),)\n NO_TCLTK = NoThanks\n endif\n-- \n2.7.0.25.gfc10eb5\n"},{"id":"275950","messageId":"xmqqwprdnxte.fsf@gitster.mtv.corp.google.com","threadId":"41178","inReplyTo":"1452704204-1928-1-git-send-email-kuleshovmail@gmail.com","subject":"Re: [PATCH] Makefile: describe XMALLOC_POISON","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-13T18:23:09Z","receivedAt":"2016-01-13T18:23:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexander Kuleshov <kuleshovmail@gmail.com> writes:\n\n> The do_xmalloc() functions may fill an allocated buffer with the\n> known value (0xA5) for debugging if we will pass the XMALLOC_POISON\n> option during build.\n>\n> This patch adds description of this option to the Makefile and\n> adds it to BASIC_CFLAGS if it was provided.\n>\n> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>\n> ---\n\nI am guessing that the message this one is a response to is\n(incomplete) v1 that I shouldn't look at, and this is v2 that is\nexpected to be useful.\n\nXMALLOC_POISON is not about \"if you are debugging the xmalloc()\",\nthough.  By filling the memory returned with a non-NUL byte, what we\nget is to catch callers that depended on the region of memory being\nNUL-filled (often happens in early in a short-lived program), so it\nis about catching more bugs in the callers of xmalloc() [*1*].\n\n>  Makefile | 8 ++++++++\n>  1 file changed, 8 insertions(+)\n>\n> diff --git a/Makefile b/Makefile\n> index f3325de..3f942b5 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -367,6 +367,10 @@ all::\n>  # Define HAVE_BSD_SYSCTL if your platform has a BSD-compatible sysctl function.\n>  #\n>  # Define HAVE_GETDELIM if your system has the getdelim() function.\n> +#\n> +# Define XMALLOC_POISON if you are debugging the xmalloc(). In a XMALLOC_POISON\n> +# build, each allocated buffer by the xmalloc() will be in known state. After\n> +# memory allocation, a buffer will be filled with '0xA5' values.\n\n\"will be in known state\" is not an interesting bit, and I do not\nthink we want people relying on the exact contents of the\nuninitialized bytes.\n\nPersonally, I feel that XMALLOC_POISON outlived its usefulness, with\nthe availability of --valgrind tests and other forms of checks\n(including static analyzers) in the modern world.  While I do not\nthink it hurts to allow people who know what they are doing to say\n\"make XMALLOC_POISON=YesPlease\", I suspect it would cause more harm\nto have a wrong description on what it is about here than the new\nmakefile knob helps them.  Because of the above, this change is\nsomewhere between \"Meh\" and \"Perhaps a bad idea\" to me.\n\nIf you have a more compelling story to tell (\"XMALLOC_POISON-enabled\nbuild allowed me to hunt down this and that kind of bug because I\ncan scan the entire address space looking for regions filled with\n0xA5 to do X\") and description based on that story is in the log\nmessage and also in the Makefile comment, on the other hand, my\nabove assessment may become vastly move positive, though.  During\nthe course of running the project since the XMALLOC_POISON was added\nin April 2006, I didn't encounter any such interesting debugging\nsession that helped me myself.\n\nThanks.\n\n>  \n> +ifdef XMALLOC_POISON\n> +\tBASIC_CFLAGS += -DXMALLOC_POISON\n> +endif\n> +\n\n\n[Footnote]\n\n*1* Another reason for choosing 0xA5 is because it is 'odd' and more\nlikely to cause bus errors on architectures that do not allow\nunaligned access when the uninitialized value is used as a pointer,\nbut its value is fairly limited.\n"}]}