Opened 3 months ago

Closed 3 months ago

#30179 closed defect (fixed)

Building with 'ALL_BUGS_ARE_FATAL' fails because of formatted assertion changes.

Reported by: gvanem Owned by:
Priority: Medium Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Normal Keywords: dgoulet-merge compilation regression
Cc: Actual Points:
Parent ID: Points:
Reviewer: nickm Sponsor:

Description

Building with ALL_BUGS_ARE_FATAL should AFAICS normally be
reserved for Coverty and/or analysers. But just for kicks, I've
added -DALL_BUGS_ARE_FATAL to my CFLAGS.

This causes errors like these (from clang using a git-checkout from today):

buf/buffers.c(531,7):  error: too few arguments to function call, expected at least 5, have 4
  if (BUG(buf->datalen >= INT_MAX))
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
..\lib/log/util_bug.h(151,70):  note: expanded from macro 'BUG'
   (tor_assertion_failed_(SHORT_FILE__,__LINE__,__func__,"!("#cond")"), \
    ~~~~~~~~~~~~~~~~~~~~~                                            ^
..\lib/log/util_bug.h(242,1):  note: 'tor_assertion_failed_' declared here
void tor_assertion_failed_(const char *fname, unsigned int line,
^
buf/buffers.c(533,7):  error: too few arguments to function call, expected at least 5, have 4
  if (BUG(buf->datalen >= INT_MAX - string_len))
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...

I've used this patch to fix it:

--- a/src/lib/log/util_bug.h 2019-04-14 03:56:41
+++ b/src/lib/log/util_bug.h 2019-04-14 11:31:23
@@ -148,7 +148,7 @@
 #define tor_assert_nonfatal_once(cond) tor_assert((cond))
 #define BUG(cond)                                                       \
   (ASSERT_PREDICT_UNLIKELY_(cond) ?                                     \
-   (tor_assertion_failed_(SHORT_FILE__,__LINE__,__func__,"!("#cond")"), \
+   (tor_assertion_failed_(SHORT_FILE__,__LINE__,__func__,"!("#cond")", NULL), \
     abort(), 1)                                                         \
    : 0)
 #elif defined(TOR_UNIT_TESTS) && defined(DISABLE_ASSERTS_IN_UNIT_TESTS)

W/o knowing what the correct fix would be.

Child Tickets

Change History (9)

comment:1 Changed 3 months ago by gvanem

There is another issue with how tor_assertf_nonfatal() is defined when
ALL_BUGS_ARE_FATAL is defined.

So my above patch is now:

--- a/src/lib/log/util_bug.h 2019-04-14 03:56:41
+++ b/src/log/util_bug.h     2019-04-14 14:45:15
@@ -143,12 +143,12 @@
 #ifdef ALL_BUGS_ARE_FATAL
 #define tor_assert_nonfatal_unreached() tor_assert(0)
 #define tor_assert_nonfatal(cond) tor_assert((cond))
-#define tor_assertf_nonfatal(cond, fmt, ...) tor_assertf(cond, fmt, ...)
+#define tor_assertf_nonfatal(cond, fmt, ...) tor_assertf(cond, fmt, ##__VA_ARGS__)
 #define tor_assert_nonfatal_unreached_once() tor_assert(0)
 #define tor_assert_nonfatal_once(cond) tor_assert((cond))
 #define BUG(cond)                                                       \
   (ASSERT_PREDICT_UNLIKELY_(cond) ?                                     \
-   (tor_assertion_failed_(SHORT_FILE__,__LINE__,__func__,"!("#cond")"), \
+   (tor_assertion_failed_(SHORT_FILE__,__LINE__,__func__,"!("#cond")", NULL), \
     abort(), 1)                                                         \
    : 0)
 #elif defined(TOR_UNIT_TESTS) && defined(DISABLE_ASSERTS_IN_UNIT_TESTS)

comment:2 Changed 3 months ago by nickm

Component: Core TorCore Tor/Tor
Milestone: Tor: 0.4.1.x-final
Status: newneeds_review

Looks like a recent bug in the formatted-assert code.

comment:3 Changed 3 months ago by asn

Reviewer: nickm

comment:4 Changed 3 months ago by nickm

Hm. There's an older bug in this code, in that it calls abort() without including stdlib.h.

comment:5 Changed 3 months ago by nickm

Summary: Building with 'ALL_BUGS_ARE_FATAL'Building with 'ALL_BUGS_ARE_FATAL' fails because of formatted assertion changes.

comment:6 Changed 3 months ago by nickm

I opened #30189 for the other issue.

comment:7 Changed 3 months ago by nickm

Status: needs_reviewmerge_ready

I put this in a branch as bug30179. It's based on bug30189_035, since otherwise there would be conflicts. PR at https://github.com/torproject/tor/pull/954 .

We can merge this once #30189 is merge_ready.

comment:8 Changed 3 months ago by nickm

Keywords: dgoulet-merge compilation regression added

comment:9 Changed 3 months ago by dgoulet

Resolution: fixed
Status: merge_readyclosed

Merged.

Note: See TracTickets for help on using tickets.