This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: assertify mn10300


On Sun, 8 May 2005, Nathan Sidwell wrote:
> Alexandre Oliva wrote:
> > The problem was not the inversion of the condition, but rather the
> > fact that, with checking disabled, the gcc_assert would be completely
> > optimized away, thus enabling the broken situation to skip the abort()
> > call and proceed to code that might fail to handle it properly.
>
> I fail to understand the state of mind that complains about the fact
> that when configured with 'please disable all consistency checks', then
> gosh darn it, all the consistency checks are removed.  It's like
> complaining that when I take the safety off a gun, well gosh darn it,
> I can shoot things.

Perhaps, I can help rationalize that state of mind.

Alex's concern stems from the blind replacement of checks of the form
"if (expr) abort", with the almost equivalent "gcc_assert (!expr)."
The potential problems arise when expr has required side-effects.

The e-mail/review of mine that Alex referred to previously is:
http://gcc.gnu.org/ml/gcc-patches/2005-04/msg00867.html
http://gcc.gnu.org/ml/gcc-patches/2005-04/msg00871.html

In the middle of Alex's patch he introduced the following line
to test that the return value of a function was always true:

+  gcc_assert (validate_change_maybe_volatile (v->insn, v->location,
+					       reg));


Clearly, if checking is disabled the compiler will optimize away
the call to validate_change_maybe_volatile which is required for
correct operation.  The correct idiom in this case is either:

+ if (!validate_change_maybe_volatile (v->insn, v->location, reg))
+   gcc_unreachable ();

or

+ bool ok = validate_change_maybe_volatile (v->insn, v->location, reg);
+ gcc_assert (ok);



Nathan has been very conscientious in his backend clean-up's, and
in the mn10300 backend, for example, there are no introductions of
gcc_assert around side-effecting expressions (which was Alex's
initial concern).



One possible improvement might be for GCC to introduce a
__builtin_sideeffects_p builtin, along the same lines as the current
__builtin_constant_p builtin, that could be used in the definition
of the gcc_assert macro to check at compile time that it doesn't
contain a side-effecting expression.

There have been examples in the past of bugs that are only visible
with --disable-checking (one reason --enable-checking=release was
introduced), but it would also be a good idea to double check that
the code generated by a --enable-checking compiler is identical to
that generated by a --disable-checking compiler, from time to time.


I don't want anyone thinking Alex's concerns are completely unfounded.
There are potential risks with incorrect use of gcc_assert, and I'm
sure that both Zack and Nathan are well aware of them.  Unfortunately,
in this thread the valid concerns have been mistaken paranoid
delusions.  Yes, if you turn off consistency checks, you no longer
test for consistency.  The concern is that without due care you may
"throw out the baby with the bath water".
gcc_assert (rest_of_compilation())

Roger
--


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]