From a4cc910a3401d25bb94ff0ecb4dc18f681c71004 Mon Sep 17 00:00:00 2001 From: David Kastrup Date: Tue, 12 May 2015 19:01:57 +0200 Subject: [PATCH] Issue 2787: Sanitize usage of -DDEBUG, -DNDEBUG and assert The compiler option -DNDEBUG is no longer being used: -DNDEBUG disables the assert function, and assert is essentially stating that the program cannot useful continue if the assertion is not met. -DNDEBUG is basically an option for compiling an application to a limited amount of ROM when aborting with a diagnostic is not preferable to crashing. This is not the case for LilyPond. So expensive debugging options now are enabled with -DDEBUG instead. There is a new configure option --enable-checking defaulting to "off" for this now. At the current point of time, setting --disable-optimising also has the effect of enabling the checks: this will be retained until Patchy has been adapted to using --enable-checking. --- .../contributor/programming-work.itexi | 35 ++++++++++++------- aclocal.m4 | 18 ++++++++-- flower/include/pqueue.hh | 2 +- flower/include/std-string.hh | 2 +- flower/include/std-vector.hh | 2 +- lily/context.cc | 6 ++-- lily/engraver.cc | 6 ++-- lily/grob-property.cc | 14 ++++---- lily/include/lily-guile-macros.hh | 2 +- lily/include/smobs.hh | 2 +- lily/lookup.cc | 2 +- lily/prob.cc | 2 +- 12 files changed, 59 insertions(+), 34 deletions(-) diff --git a/Documentation/contributor/programming-work.itexi b/Documentation/contributor/programming-work.itexi index a02910b768..28f6e8d4a6 100644 --- a/Documentation/contributor/programming-work.itexi +++ b/Documentation/contributor/programming-work.itexi @@ -913,17 +913,21 @@ The GNU debugger, gdb, is the principal tool for debugging C++ code. @subheading Compiling LilyPond for use with gdb In order to use gdb with LilyPond, it is necessary to compile -LilyPond with debugging information. This is accomplished by running -the following commands in the main LilyPond source directory. +LilyPond with debugging information. This is the current default +mode of compilation. Often debugging becomes more complicated +when the compiler has optimised variables and function calls away. +In that case it may be helpful to run the following command in the +main LilyPond source directory: @example -./configure --disable-optimising +./configure --disable-optimising make @end example -This will create a version of LilyPond containing debugging -information that will allow the debugger to tie the source code -to the compiled code. +This will create a version of LilyPond with minimal optimization +which will allow the debugger to access all variables and step +through the source code in-order. It may not accurately reproduce +bugs encountered with the optimized version, however. You should not do @var{make install} if you want to use a debugger with LilyPond. The @var{make install} command will strip debugging @@ -1204,8 +1208,15 @@ number of different platforms: In order for the Graphviz tool to work, config.make must be modified. It is probably a good idea to first save a copy of config.make under -a different name. Then, edit config.make by removing every occurrence -of @option{-DNDEBUG}. +a different name. + +In order to have the required functionality available, LilyPond +needs to be compiled with the option @option{-DDEBUG}. You can +achieve this by configuring with + +@example +./configure --enable-checking +@end example @item Rebuilding LilyPond @@ -1253,10 +1264,10 @@ dot -Tpdf graphviz.log > graphviz.pdf The pdf file can then be viewed with any pdf viewer. -When compiled without @option{-DNDEBUG}, lilypond may run slower -than normal. The original configuration can be restored by either -renaming the saved copy of @code{config.make} or rerunning -@code{configure}. Then rebuild lilypond with +When compiled with @option{-DDEBUG}, lilypond may run slower +than normal. The original configuration can be restored by rerunning +@code{./configure} with @option{--disable-checking}. Then +rebuild lilypond with @example make -C lily clean && make -C lily diff --git a/aclocal.m4 b/aclocal.m4 index 7ed3151246..48d0b77b74 100644 --- a/aclocal.m4 +++ b/aclocal.m4 @@ -173,6 +173,7 @@ AC_DEFUN(STEPMAKE_COMPILE_BEFORE, [ CXXFLAGS=${CXXFLAGS-$CFLAGS} LDFLAGS=${LDFLAGS-""} optimise_b=yes + checks_b=no profile_b=no debug_b=yes pipe_b=yes @@ -182,6 +183,11 @@ AC_DEFUN(STEPMAKE_COMPILE_BEFORE, [ [compile with debugging info. Default: on])], [debug_b=$enableval]) + AC_ARG_ENABLE(checking, + [AS_HELP_STRING([--enable-checking], + [compile with expensive run-time checks. Default: off])], + [checks_b=$enableval]) + AC_ARG_ENABLE(optimising, [AS_HELP_STRING([--enable-optimising], [compile with optimising. Default: on])], @@ -198,9 +204,17 @@ AC_DEFUN(STEPMAKE_COMPILE_BEFORE, [ [pipe_b=$enableval]) if test "$optimise_b" = yes; then - AC_DEFINE(NDEBUG) - DEFINES="$DEFINES -DNDEBUG" OPTIMIZE=" -O2 -finline-functions" + # following two lines are compatibility while Patchy has not + # yet learnt about --enable-checking. But once it has, we + # don't want -DDEBUG twice, so we omit it here if it is going + # to get added anyway later. + elif test "$checks_b" != yes; then + DEFINES="$DEFINES -DDEBUG" + fi + + if test "$checks_b" = yes; then + DEFINES="$DEFINES -DDEBUG" fi if test $profile_b = yes; then diff --git a/flower/include/pqueue.hh b/flower/include/pqueue.hh index 39e0cbe743..6f1ebc53b6 100644 --- a/flower/include/pqueue.hh +++ b/flower/include/pqueue.hh @@ -69,7 +69,7 @@ public: } void OK () const { -#ifndef NDEBUG +#ifdef DEBUG for (vsize i = 2; i <= size (); i++) assert (compare (elt (i / 2), elt (i)) <= 0); #endif diff --git a/flower/include/std-string.hh b/flower/include/std-string.hh index ea65fe042e..651f3403a1 100644 --- a/flower/include/std-string.hh +++ b/flower/include/std-string.hh @@ -27,7 +27,7 @@ /* leads to dubious crashes - libstdc++ bug? */ -#ifndef NDEBUG +#ifdef DEBUG #define _GLIBCXX_DEBUG 1 #endif #endif diff --git a/flower/include/std-vector.hh b/flower/include/std-vector.hh index bb2103235e..fae955854b 100644 --- a/flower/include/std-vector.hh +++ b/flower/include/std-vector.hh @@ -25,7 +25,7 @@ /* leads to dubious crashes - libstdc++ bug? */ -#ifndef NDEBUG +#ifdef DEBUG #define _GLIBCXX_DEBUG 1 #endif #endif diff --git a/lily/context.cc b/lily/context.cc index 4c0caac040..348d307479 100644 --- a/lily/context.cc +++ b/lily/context.cc @@ -438,7 +438,7 @@ Context::get_default_interpreter (const string &context_id) Context * Context::where_defined (SCM sym, SCM *value) const { -#ifndef NDEBUG +#ifdef DEBUG if (profile_property_accesses) note_property_access (&context_property_lookup_table, sym); #endif @@ -454,7 +454,7 @@ Context::where_defined (SCM sym, SCM *value) const bool Context::here_defined (SCM sym, SCM *value) const { -#ifndef NDEBUG +#ifdef DEBUG if (profile_property_accesses) note_property_access (&context_property_lookup_table, sym); #endif @@ -468,7 +468,7 @@ Context::here_defined (SCM sym, SCM *value) const SCM Context::internal_get_property (SCM sym) const { -#ifndef NDEBUG +#ifdef DEBUG if (profile_property_accesses) note_property_access (&context_property_lookup_table, sym); #endif diff --git a/lily/engraver.cc b/lily/engraver.cc index fd352d836d..76e1ef6f58 100644 --- a/lily/engraver.cc +++ b/lily/engraver.cc @@ -86,7 +86,7 @@ Engraver::Engraver () { } -#ifndef NDEBUG +#ifdef DEBUG static SCM creation_callback = SCM_EOL; LY_DEFINE (ly_set_grob_creation_callback, "ly:set-grob-creation-callback", 1, 0, 0, (SCM cb), @@ -112,7 +112,7 @@ Engraver::internal_make_grob (SCM symbol, int line, char const *fun) { -#ifdef NDEBUG +#ifndef DEBUG (void)file; (void)line; (void)fun; @@ -135,7 +135,7 @@ Engraver::internal_make_grob (SCM symbol, assert (grob); announce_grob (grob, cause); -#ifndef NDEBUG +#ifdef DEBUG if (ly_is_procedure (creation_callback)) scm_apply_0 (creation_callback, scm_list_n (grob->self_scm (), scm_from_utf8_string (file), diff --git a/lily/grob-property.cc b/lily/grob-property.cc index 2812643c3e..7f7368df69 100644 --- a/lily/grob-property.cc +++ b/lily/grob-property.cc @@ -26,7 +26,7 @@ Protected_scm grob_property_callback_stack (SCM_EOL); extern bool debug_property_callbacks; -#ifndef NDEBUG +#ifdef DEBUG static void print_property_callback_stack () { @@ -77,7 +77,7 @@ Grob::instrumented_set_property (SCM sym, SCM v, int line, char const *fun) { -#ifndef NDEBUG +#ifdef DEBUG if (ly_is_procedure (modification_callback)) scm_apply_0 (modification_callback, scm_list_n (self_scm (), @@ -137,7 +137,7 @@ Grob::internal_set_value_on_alist (SCM *alist, SCM sym, SCM v) SCM Grob::internal_get_property_data (SCM sym) const { -#ifndef NDEBUG +#ifdef DEBUG if (profile_property_accesses) note_property_access (&grob_property_lookup_table, sym); #endif @@ -166,7 +166,7 @@ Grob::internal_get_property (SCM sym) const { SCM val = get_property_data (sym); -#ifndef NDEBUG +#ifdef DEBUG if (scm_is_eq (val, ly_symbol2scm ("calculation-in-progress"))) { programming_error (to_string ("cyclic dependency: calculation-in-progress encountered for #'%s (%s)", @@ -232,7 +232,7 @@ Grob::try_callback_on_alist (SCM *alist, SCM sym, SCM proc) */ *alist = scm_assq_set_x (*alist, sym, marker); -#ifndef NDEBUG +#ifdef DEBUG if (debug_property_callbacks) grob_property_callback_stack = scm_cons (scm_list_3 (self_scm (), sym, proc), grob_property_callback_stack); #endif @@ -247,7 +247,7 @@ Grob::try_callback_on_alist (SCM *alist, SCM sym, SCM proc) false, 0, 0); } -#ifndef NDEBUG +#ifdef DEBUG if (debug_property_callbacks) grob_property_callback_stack = scm_cdr (grob_property_callback_stack); #endif @@ -261,7 +261,7 @@ Grob::try_callback_on_alist (SCM *alist, SCM sym, SCM proc) } else { -#ifndef NDEBUG +#ifdef DEBUG if (ly_is_procedure (cache_callback)) scm_apply_0 (cache_callback, scm_list_n (self_scm (), diff --git a/lily/include/lily-guile-macros.hh b/lily/include/lily-guile-macros.hh index 10520d6dc9..ef48650353 100644 --- a/lily/include/lily-guile-macros.hh +++ b/lily/include/lily-guile-macros.hh @@ -215,7 +215,7 @@ void ly_check_name (const string &cxx, const string &fname); #define set_object(x, y) internal_set_object (ly_symbol2scm (x), y) #define del_property(x) internal_del_property (ly_symbol2scm (x)) -#ifndef NDEBUG +#ifdef DEBUG /* TODO: include modification callback support here, perhaps through intermediate Grob::instrumented_set_property( .. __LINE__ ). diff --git a/lily/include/smobs.hh b/lily/include/smobs.hh index eb8bb482f0..7097bb3d48 100644 --- a/lily/include/smobs.hh +++ b/lily/include/smobs.hh @@ -333,7 +333,7 @@ public: // least some, so they are apparently not protected in spite of being // included in the GC scans. So it would appear that scanning smobs // is not equivalent to marking them. Ugh. -#if !defined(NDEBUG) && !GUILEV2 +#if defined(DEBUG) && !GUILEV2 #define ASSERT_LIVE_IS_ALLOWED(arg) \ do { \ static parsed_dead pass_here; \ diff --git a/lily/lookup.cc b/lily/lookup.cc index 6bd55bbc7d..280b85bcfa 100644 --- a/lily/lookup.cc +++ b/lily/lookup.cc @@ -244,7 +244,7 @@ Lookup::round_filled_polygon (vector const &points, const Real epsilon = 0.01; -#ifndef NDEBUG +#ifdef DEBUG /* remove consecutive duplicate points */ for (vsize i = 0; i < points.size (); i++) { diff --git a/lily/prob.cc b/lily/prob.cc index 37dad6246b..a7245c653f 100644 --- a/lily/prob.cc +++ b/lily/prob.cc @@ -141,7 +141,7 @@ Prob::print_smob (SCM port, scm_print_state *) SCM Prob::internal_get_property (SCM sym) const { -#ifndef NDEBUG +#ifdef DEBUG if (profile_property_accesses) note_property_access (&prob_property_lookup_table, sym); #endif -- 2.39.5