Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise our assertion and bug macros to work with -Wparentheses #337

Merged
merged 2 commits into from Oct 15, 2018

Conversation

Labels
None yet
Projects
None yet
4 participants
@nmathewson
Copy link
Contributor

@nmathewson nmathewson commented Sep 14, 2018

On GCC and Clang, there's a feature to warn you about bad
conditionals like "if (a = b)", which should be "if (a == b)".
However, they don't warn you if there are extra parentheses around
"a = b".

Unfortunately, the tor_assert() macro and all of its kin have been
passing their inputs through stuff like PREDICT_UNLIKELY(expr) or
PREDICT_UNLIKELY(!(expr)), both of which expand to stuff with more
parentheses around "expr", thus suppressing these warnings.

To fix this, this patch introduces new macros that do not wrap
expr. They're only used when GCC or Clang is enabled (both define
GNUC), since they require GCC's "({statement expression})"
syntax extension. They're only used when we're building the
unit-test variant of the object files, since they suppress the
branch-prediction hints.

I've confirmed that tor_assert(), tor_assert_nonfatal(),
tor_assert_nonfatal_once(), BUG(), and IF_BUG_ONCE() all now give
compiler warnings when their argument is an assignment expression.

Fixes bug 27709.

Bugfix on 0.0.6, where we first introduced the "tor_assert()" macro.

On GCC and Clang, there's a feature to warn you about bad
conditionals like "if (a = b)", which should be "if (a == b)".
However, they don't warn you if there are extra parentheses around
"a = b".

Unfortunately, the tor_assert() macro and all of its kin have been
passing their inputs through stuff like PREDICT_UNLIKELY(expr) or
PREDICT_UNLIKELY(!(expr)), both of which expand to stuff with more
parentheses around "expr", thus suppressing these warnings.

To fix this, this patch introduces new macros that do not wrap
expr.  They're only used when GCC or Clang is enabled (both define
__GNUC__), since they require GCC's "({statement expression})"
syntax extension.  They're only used when we're building the
unit-test variant of the object files, since they suppress the
branch-prediction hints.

I've confirmed that tor_assert(), tor_assert_nonfatal(),
tor_assert_nonfatal_once(), BUG(), and IF_BUG_ONCE() all now give
compiler warnings when their argument is an assignment expression.

Fixes bug 27709.

Bugfix on 0.0.6, where we first introduced the "tor_assert()" macro.
@coveralls
Copy link

@coveralls coveralls commented Sep 14, 2018

Coverage Status

Coverage increased (+0.009%) to 49.973% when pulling 5e582c7 on nmathewson:bug27709_029 into c02f2d9 on torproject:maint-0.2.9.

src/common/util_bug.h Show resolved Hide resolved
src/common/util_bug.h Show resolved Hide resolved
@torproject-pusher torproject-pusher merged commit 5e582c7 into torproject:maint-0.2.9 Oct 15, 2018
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment