Trac issueshttps://gitlab.torproject.org/legacy/trac/-/issues2020-06-13T14:46:50Zhttps://gitlab.torproject.org/legacy/trac/-/issues/16343Address of struct member is always non-NULL in SSL_SESSION_get_master_key in ...2020-06-13T14:46:50ZteorAddress of struct member is always non-NULL in SSL_SESSION_get_master_key in f90a704f1258clang complains that the address of a struct member is always non-NULL in SSL_SESSION_get_master_key in commit f90a704f1258.
Instead, I suggest we check `SSL_SESSION *s` and `uint8_t *out` are non-NULL right before we dereference them.clang complains that the address of a struct member is always non-NULL in SSL_SESSION_get_master_key in commit f90a704f1258.
Instead, I suggest we check `SSL_SESSION *s` and `uint8_t *out` are non-NULL right before we dereference them.Tor: 0.2.7.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/15817Allow clang runtime sanitizers to be used on tor unit tests2020-06-13T14:45:39ZteorAllow clang runtime sanitizers to be used on tor unit testsThe clang runtime address sanitizer causes the memwipe and backtrace tests to fail, because it catches the undefined behavior that is invoked as an unavoidable part of these tests.
This issue can be resolved by:
1. blacklisting the invo...The clang runtime address sanitizer causes the memwipe and backtrace tests to fail, because it catches the undefined behavior that is invoked as an unavoidable part of these tests.
This issue can be resolved by:
1. blacklisting the involved functions so the sanitizer doesn't check them, by adding to the command line `-fsanitize-blacklist=sanitize_blacklist.txt` containing:
```
# test-memwipe.c checks if a freed buffer was properly wiped
fun:vmemeq
# test_bt_cl.c stores to a NULL pointer to trigger a crash
fun:crash
# we need to exempt the entire file, otherwise address sanitizer munges
# the expected output
src:test_bt_cl.c
```
2. allowing the backtrace handler to catch SIGSEGV (rather than address sanitizer), by setting the environmental variable `ASAN_OPTIONS=allow_user_segv_handler=1`
The attached file is a sanitizer blacklist file which documents this resolution, and implements the blacklisting (but not the environmental variable).
Nick, can we put this in contrib?
I think that's the best way to deal with the special case of "clang users running tests under AddressSanitizer"Tor: 0.2.7.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/14798Avoid calling SMARTLIST_FOREACH on a NULL smartlist in tests2020-06-13T14:42:36ZteorAvoid calling SMARTLIST_FOREACH on a NULL smartlist in testsTwo of the test functions could possibly call SMARTLIST_FOREACH on a NULL smartlist.
This branch fixes the issue:
**Repository:** https://github.com/teor2345/tor.git
**Branch:** avoid-NULL-smartlist-foreach
Check if each smartlist ...Two of the test functions could possibly call SMARTLIST_FOREACH on a NULL smartlist.
This branch fixes the issue:
**Repository:** https://github.com/teor2345/tor.git
**Branch:** avoid-NULL-smartlist-foreach
Check if each smartlist is NULL before calling SMARTLIST_FOREACH on it.
Bug discovered by the clang static analyzer.
Apple clang 600.0.56 (LLVM 3.5svn) on x86_64-apple-darwin14.1.0.Tor: 0.2.6.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/14296Avoid redundant check for NULL defport->unix_addr in config.c2020-06-13T14:42:00ZteorAvoid redundant check for NULL defport->unix_addr in config.cAnything in the middle of a structure will always be non-null.
clang 3.6.0 warns on this because it is redundant.
See my branch **config-remove-redundant-assert** in my **teor2345** github.Anything in the middle of a structure will always be non-null.
clang 3.6.0 warns on this because it is redundant.
See my branch **config-remove-redundant-assert** in my **teor2345** github.Tor: 0.2.6.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/21496Check string passed to extrainfo_parse_entry_from_string2020-06-13T15:06:27ZteorCheck string passed to extrainfo_parse_entry_from_stringscan-build reports the following out of bounds access, because it doesn't know that s is always non-NULL. But we should guard against it being NULL.
extrainfo_parse_entry_from_string:
```
+ if (BUG(!s))
+ return;
while (end > s+2...scan-build reports the following out of bounds access, because it doesn't know that s is always non-NULL. But we should guard against it being NULL.
extrainfo_parse_entry_from_string:
```
+ if (BUG(!s))
+ return;
while (end > s+2 && *(end-1) == '\n' && *(end-2) == '\n')
```Tor: 0.3.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/8598clang warning in circuitbuild.c switch statement2020-06-13T14:28:31ZNick Mathewsonclang warning in circuitbuild.c switch statementIn maint-0.2.4, building with clang 3.2 on fedora, I get:
```
src/or/circuitbuild.c:1946:11: error: enumeration values 'PATH_STATE_NEW_CIRC',
'PATH_STATE_BUILD_ATTEMPTED', and 'PATH_STATE_ALREADY_COUNTED' not
explicitly hand...In maint-0.2.4, building with clang 3.2 on fedora, I get:
```
src/or/circuitbuild.c:1946:11: error: enumeration values 'PATH_STATE_NEW_CIRC',
'PATH_STATE_BUILD_ATTEMPTED', and 'PATH_STATE_ALREADY_COUNTED' not
explicitly handled in switch [-Werror,-Wswitch-enum]
switch (ocirc->path_state) {
```
I have a fix for this; just adding this ticket so I can give it a bug number.Tor: 0.2.4.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/13577Fix clang compilation warnings in tor, trunnel2020-06-13T14:39:46ZteorFix clang compilation warnings in tor, trunnel My branch silence-warnings on github contains fixes to silence clang warnings under --enable-expensive-hardening, including:
+ implicit truncation of 64 bit values to 32 bit;
+ const char assignment to self;
+ tautological compare... My branch silence-warnings on github contains fixes to silence clang warnings under --enable-expensive-hardening, including:
+ implicit truncation of 64 bit values to 32 bit;
+ const char assignment to self;
+ tautological compare; and
+ additional parentheses around equality tests. (gcc uses these to
silence assignment, so clang warns when they're present in an
equality test. But we need to use extra parentheses in macros to
isolate them from other code).
Branch: silence-warnings
Repository: https://github.com/teor2345/tor.git
Nick, do you want the trunnel patch submitted upstream?Tor: 0.2.6.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/14001Fix clang warning, incorrect comment about IPv6, and apparent typo in log buf...2020-06-13T14:41:15ZteorFix clang warning, incorrect comment about IPv6, and apparent typo in log buffer sizeclang gives a warning about the address of an array mid-way through a structure always being non-NULL. AFAICT, this is true, so I've disabled the check.
A comment about an IPv6 address string incorrectly refers to an IPv4 address format...clang gives a warning about the address of an array mid-way through a structure always being non-NULL. AFAICT, this is true, so I've disabled the check.
A comment about an IPv6 address string incorrectly refers to an IPv4 address format.
A log buffer is sized 10024 rather than 10240.
I'll post a branch soon with fixes to these.Tor: 0.2.6.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/21495Fix scan-build warning: true and false branches are identical2020-06-13T15:06:26ZteorFix scan-build warning: true and false branches are identicalThis is fine, we can rewrite it, or tell scan-build to ignore it:
dirvote_get_intermediate_param_value:
```
else if (BUG(n_found > 1))
return default_val;
else
return default_val;
```
I blame boolean macros with side-effects.This is fine, we can rewrite it, or tell scan-build to ignore it:
dirvote_get_intermediate_param_value:
```
else if (BUG(n_found > 1))
return default_val;
else
return default_val;
```
I blame boolean macros with side-effects.Tor: 0.3.2.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/21494Fix scan-build warnings about assigning to char**2020-06-13T15:06:26ZteorFix scan-build warnings about assigning to char**scan-build complains about:
tor_check_port_forwarding:
```
const char **argv; /* cli arguments */
...
argv = tor_calloc(args_n, sizeof(char *));
```
and:
process_environment_make:
```
process_environment_t *env = tor_malloc_zero(sizeof(p...scan-build complains about:
tor_check_port_forwarding:
```
const char **argv; /* cli arguments */
...
argv = tor_calloc(args_n, sizeof(char *));
```
and:
process_environment_make:
```
process_environment_t *env = tor_malloc_zero(sizeof(process_environment_t));
...
env->unixoid_environment_block = tor_calloc(n_env_vars + 1, sizeof(char *));
```
This is fine, and maybe there's a cast or magic comment we could use to make the warnings go away.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/21497Fix scan-build warnings: memory accesses2020-06-13T15:06:27ZteorFix scan-build warnings: memory accessesThese should all be ok.
router_get_hash_impl_helper:
```
- if (start != s && *(start-1) != '\n') {
+ if (start > s && *(start-1) != '\n') {
```
read_escaped_data:
This is just a false positive, we should add ignore comments:
```
if (...These should all be ok.
router_get_hash_impl_helper:
```
- if (start != s && *(start-1) != '\n') {
+ if (start > s && *(start-1) != '\n') {
```
read_escaped_data:
This is just a false positive, we should add ignore comments:
```
if (n_to_copy && *(next-1) == '\r')
```
format_win_cmdline_argument:
These are just false positives, we should add ignore comments around the trailing quote and terminator assignments:
```
formatted_arg[i++] = '"';
```
```
formatted_arg[i] = '\0';
```
unescape_string:
These are just false positives, we should add ignore comments:
```
*out++ = *cp++;
```
```
*out++ = cp[1];
```
```
*out++ = (char)n;
```
```
*out++ = ((x1<<4) + x2);
```
```
case 'n': *out++ = '\n'; cp += 2; break;
case 'r': *out++ = '\r'; cp += 2; break;
case 't': *out++ = '\t'; cp += 2; break;
```
```
*out = '\0';
```
tor_escape_str_for_pt_args:
These are just false positives, we should add ignore comments:
```
*new_cp = '\0'; /* NUL-terminate the new string */
```
```
*new_cp++ = *string++;
```
process_environment_make:
This is just a false positive, we should add ignore comments:
```
memcpy(cp, s, slen+1);
```Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/19378Remove completely irrelevant warnings from the clang list2020-06-13T14:58:37ZNick MathewsonRemove completely irrelevant warnings from the clang listSome of the warnings in configure.ac that I added as part of #19180 are probably c++-only, or objc-only, or openmp-only. We should disable those.Some of the warnings in configure.ac that I added as part of #19180 are probably c++-only, or objc-only, or openmp-only. We should disable those.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/30225Run clang's scan-build in Tor's CI2020-06-13T15:40:52ZteorRun clang's scan-build in Tor's CIThere can be a few days between merging and a coverity build.
So we could also use clang's scan-build in CI, to get immediate feedback.
Code that passes scan-build should have less coverity defects.There can be a few days between merging and a coverity build.
So we could also use clang's scan-build in CI, to get immediate feedback.
Code that passes scan-build should have less coverity defects.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/14189Silence implicit uint64_t to size_t conversion warnings in clang on 32-bit OS X2020-06-13T14:41:46ZteorSilence implicit uint64_t to size_t conversion warnings in clang on 32-bit OS XThe out of memory checking code performs a calculation based on the difference between the memory used (a `size_t`) and the memory limit (a `uint64_t`).
This causes warnings in clang about implicit conversions losing precision. To ensur...The out of memory checking code performs a calculation based on the difference between the memory used (a `size_t`) and the memory limit (a `uint64_t`).
This causes warnings in clang about implicit conversions losing precision. To ensure this isn't happening, I assert `MaxMemInQueues <= SIZE_T_MAX` before casting.
Branch will be posted after I have the bug number.Tor: 0.2.6.x-final