Michał Kępień [Thu, 26 Mar 2020 10:03:52 +0000 (11:03 +0100)]
Include compiler name in all build/test job names
Most build/test job names already contain a "clang", "gcc", or "msvc"
prefix which indicates the compiler used for a given job. Apply that
naming convention to all build/test job names.
Michał Kępień [Thu, 26 Mar 2020 10:03:52 +0000 (11:03 +0100)]
Refactor TSAN unit test job definitions
Multiple YAML keys have identical values for both TSAN unit test job
definitions. Extract these common keys to a YAML anchor and use it in
TSAN unit test job definitions to reduce code duplication.
Michał Kępień [Thu, 26 Mar 2020 10:03:52 +0000 (11:03 +0100)]
Run "kyua report-html" for TSAN unit test jobs
Definitions of jobs running unit tests under TSAN contain an
"after_script" YAML key. Since the "unit_test_job" anchor is included
in those job definitions before "after_script" is defined, the
job-specific value of that key overrides the one defined in the included
anchor. This prevents "kyua report-html" from being run for TSAN unit
test jobs. Moving the invocation of "kyua report-html" to the "script"
key in the "unit_test_job" anchor is not acceptable as it would cause
the exit code of that command to determine the result of all unit test
jobs and we need that to be the exit code of "make unit". Instead, add
"kyua report-html" invocations to the "after_script" key of TSAN unit
test job definitions to address the problem without affecting other job
definitions.
Michał Kępień [Thu, 26 Mar 2020 10:03:52 +0000 (11:03 +0100)]
Refactor TSAN system test job definitions
Multiple YAML keys have identical values for both TSAN system test job
definitions. Extract these common keys to a YAML anchor and use it in
TSAN system test job definitions to reduce code duplication.
Michał Kępień [Thu, 26 Mar 2020 10:03:52 +0000 (11:03 +0100)]
Drop "before_script" key from TSAN job definitions
Both "system_test_job" and "unit_test_job" YAML anchors contain a
"before_script" key. TSAN job definitions first specify their own value
of the "before_script" key and then include the aforementioned YAML
anchors, which results in the value of the "before_script" key being
overridden with the value specified by the included anchor. Given this,
remove "before_script" definitions specific to TSAN jobs as they serve
no practical purpose.
Michał Kępień [Thu, 26 Mar 2020 10:03:52 +0000 (11:03 +0100)]
Define TSAN options in a global variable
All assignments for the TSAN_OPTIONS variable are identical across the
entire .gitlab-ci.yml file. Define a global TSAN_OPTIONS_COMMON
variable and use it in job definitions to reduce code duplication.
Ondřej Surý [Tue, 24 Mar 2020 08:43:45 +0000 (09:43 +0100)]
Adjust the GitLab CI jobs to match the new images
The custom builds (oot, asan, tsan) were mostly built using Debian sid
amd64 image. The problem was that this image broke too easily, because
it's Debian "unstable" after all.
This commit introduces "base_image" that should be most stable with
extra bits on top (clang, coccinelle, cppcheck, ...). Currently, that
would be Debian buster amd64.
Other changes introduced by this commit:
* Change the default clang version to 10
* Run both ASAN and TSAN with both gcc and clang compilers
* Remove Clang Debian stretch i386 job
Ondřej Surý [Wed, 25 Mar 2020 16:25:45 +0000 (17:25 +0100)]
Fix 'Dereference of null pointer' from scan-build-10
These are mostly false positives, the clang-analyzer FAQ[1] specifies
why and how to fix it:
> The reason the analyzer often thinks that a pointer can be null is
> because the preceding code checked compared it against null. So if you
> are absolutely sure that it cannot be null, remove the preceding check
> and, preferably, add an assertion as well.
The 4 warnings reported are:
dnssec-cds.c:781:4: warning: Access to field 'base' results in a dereference of a null pointer (loaded from variable 'buf')
isc_buffer_availableregion(buf, &r);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/builds/isc-projects/bind9/lib/isc/include/isc/buffer.h:996:36: note: expanded from macro 'isc_buffer_availableregion'
^
/builds/isc-projects/bind9/lib/isc/include/isc/buffer.h:821:16: note: expanded from macro 'ISC__BUFFER_AVAILABLEREGION'
(_r)->base = isc_buffer_used(_b); \
^~~~~~~~~~~~~~~~~~~
/builds/isc-projects/bind9/lib/isc/include/isc/buffer.h:152:29: note: expanded from macro 'isc_buffer_used'
((void *)((unsigned char *)(b)->base + (b)->used)) /*d*/
^~~~~~~~~
1 warning generated.
--
byname_test.c:308:34: warning: Access to field 'fwdtable' results in a dereference of a null pointer (loaded from variable 'view')
RUNTIME_CHECK(dns_fwdtable_add(view->fwdtable, dns_rootname,
^~~~~~~~~~~~~~
/builds/isc-projects/bind9/lib/isc/include/isc/util.h:318:52: note: expanded from macro 'RUNTIME_CHECK'
^~~~
/builds/isc-projects/bind9/lib/isc/include/isc/error.h:50:21: note: expanded from macro 'ISC_ERROR_RUNTIMECHECK'
((void)(ISC_LIKELY(cond) || \
^~~~
/builds/isc-projects/bind9/lib/isc/include/isc/likely.h:23:43: note: expanded from macro 'ISC_LIKELY'
^
1 warning generated.
--
./rndc.c:255:6: warning: Dereference of null pointer (loaded from variable 'host')
if (*host == '/') {
^~~~~
1 warning generated.
--
./main.c:1254:9: warning: Access to field 'sctx' results in a dereference of a null pointer (loaded from variable 'named_g_server')
sctx = named_g_server->sctx;
^~~~~~~~~~~~~~~~~~~~
1 warning generated.
Ondřej Surý [Wed, 25 Mar 2020 16:00:07 +0000 (17:00 +0100)]
Fix 'Dead nested assignment's from scan-build-10
The 3 warnings reported are:
os.c:872:7: warning: Although the value stored to 'ptr' is used in the enclosing expression, the value is never actually read from 'ptr'
if ((ptr = strtok_r(command, " \t", &last)) == NULL) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
--
rpz.c:1117:10: warning: Although the value stored to 'zbits' is used in the enclosing expression, the value is never actually read from 'zbits'
return (zbits &= x);
^ ~
1 warning generated.
--
openssleddsa_link.c:532:10: warning: Although the value stored to 'err' is used in the enclosing expression, the value is never actually read from 'err'
while ((err = ERR_get_error()) != 0) {
^ ~~~~~~~~~~~~~~~
1 warning generated.
Witold Kręcicki [Fri, 20 Mar 2020 10:55:10 +0000 (11:55 +0100)]
netmgr refactoring: use generic functions when operating on sockets.
tcpdns used transport-specific functions to operate on the outer socket.
Use generic ones instead, and select the proper call in netmgr.c.
Make the missing functions (e.g. isc_nm_read) generic and add type-specific
calls (isc__nm_tcp_read). This is the preparation for netmgr TLS layer.
Ondřej Surý [Tue, 24 Mar 2020 12:56:29 +0000 (13:56 +0100)]
Remove Debian 8 ("Jessie") from the GitLab CI
There are several reason why remove Debian 8 from the CI:
* Debian 8 ("jessie") has been superseded by Debian 9 ("stretch").
* Regular security support updates have been discontinued as of
June 17th, 2018.
* Jessie LTS is supported from 17th June 2018 to June 30, 2020.
In other words, it's no longer officially supported by Debian security
team, but by the volunteer/paid contributor composed LTS team. And the
release will be discontinued in three months from now. We can use the
freed CI resources to bring new platforms or just to make the jobs run a
bit faster.
Michał Kępień [Tue, 24 Mar 2020 06:36:24 +0000 (07:36 +0100)]
Update release checklist (BIND_BASELINE_VERSION)
The reference BIND version used in the ABI check CI job is not
determined automatically - it needs to be updated after each BIND
release. Reflect that fact in the release checklist to make sure the
ABI check CI job is always comparing current code with the latest BIND
release on a given branch.
Michal Nowak [Wed, 29 Jan 2020 14:56:44 +0000 (15:56 +0100)]
Enhance unit test debugging
When unit test fails, core file is created. Kyua's 'debug' command can
run GDB on it and provide backtrace. Unfortunately Kyua is picky about
location of these core files we opt to use custom Kyua fork and copy
core files from Kyua working directory to source tree and make it
available in GitLab.
Ondřej Surý [Wed, 18 Mar 2020 14:33:52 +0000 (15:33 +0100)]
Fix the tkey system test to allow parallel run
The tkey test was not adapted to dynamic ports, so we had to run it in
sequence. This commit adds support for dynamic ports, and also makes
all the scripts shellcheck clean.
Ondřej Surý [Wed, 18 Mar 2020 14:30:56 +0000 (15:30 +0100)]
Fix the eddsa system test to allow parallel run
The eddsa test was not adapted to dynamic ports, so we had to run it in
sequence. This commit adds support for dynamic ports, and also makes
all the scripts shellcheck clean.
Ondřej Surý [Wed, 18 Mar 2020 14:24:15 +0000 (15:24 +0100)]
Fix the ecdsa system test to allow parallel run
The ecdsa test was not adapted to dynamic ports, so we had to run it in
sequence. This commit adds support for dynamic ports, and also makes
all the scripts shellcheck clean.
Ondřej Surý [Tue, 7 Aug 2018 14:46:53 +0000 (16:46 +0200)]
Rename MAKE environment variable to MAKE_COMMAND
The environment variable MAKE has been replaced with MAKE_COMMAND,
because overriding MAKE variable also changed the definition of the MAKE
inside the Makefiles, and we want only a single wrapper around the whole
build process.
Previously, setting `MAKE` to `bear make` meant that `bear make` would
be run at every nested make invocation, which messed up the upcoming
automake transition as compile_commands.json would be generated in every
subdirectory instead of just having one central file at the top of the
build tree.
Ondřej Surý [Tue, 7 Aug 2018 14:46:53 +0000 (16:46 +0200)]
Replace dependencies+needs with needs+artifacts in GitLabCI config
All jobs now use solely the newer needs configuration to declare
dependencies between jobs:
needs:
- job: <foo>
artifacts: true
instead of combination of dependencies and needs which is deprecated.
This change completely unbundles the stages (alas the stages still needs
to stay because the job graph has to stay acyclic between the stages).
Ondřej Surý [Fri, 13 Mar 2020 14:16:14 +0000 (15:16 +0100)]
Use isc_rwlock to lock .logconfig member of isc_log_t
In isc_log_woudlog() the .logconfig member of isc_log_t structure was
accessed unlocked on the merit that there could be just a race when
.logconfig would be NULL, so the message would not be logged. This
turned not to be true, as there's also data race deeper. The accessed
isc_logconfig_t object could be in the middle of destruction, so the
pointer would be still non-NULL, but the structure members could point
to a chunk of memory no longer belonging to the object. Since we are
only accessing integer types (the log level), this would never lead to
a crash, it leads to memory access to memory area no longer belonging to
the object and this a) wrong, b) raises a red flag in thread-safety tools.
Mark Andrews [Wed, 18 Mar 2020 03:17:55 +0000 (14:17 +1100)]
Refactor the isc_log API so it cannot fail on memory failures
The isc_mem API now crashes on memory allocation failure, and this is
the next commit in series to cleanup the code that could fail before,
but cannot fail now, e.g. isc_result_t return type has been changed to
void for the isc_log API functions that could only return ISC_R_SUCCESS.
Ondřej Surý [Fri, 13 Mar 2020 07:38:37 +0000 (08:38 +0100)]
Add C11 localtime_r and gmtime_r shims for Windows
On Windows, C11 localtime_r() and gmtime_r() functions are not
available. While localtime() and gmtime() functions are already thread
safe because they use Thread Local Storage, it's quite ugly to #ifdef
around every localtime_r() and gmtime_r() usage to make the usage also
thread-safe on POSIX platforms.
The commit adds wrappers around Windows localtime_s() and gmtime_s()
functions.
NOTE: The implementation of localtime_s and gmtime_s in Microsoft CRT
are incompatible with the C standard since it has reversed parameter
order and errno_t return type.
Ondřej Surý [Thu, 12 Mar 2020 09:20:37 +0000 (10:20 +0100)]
Stop leaking OpenSSL types and defines in the isc/md.h
The <isc/md.h> header directly included <openssl/evp.h> header which
enforced all users of the libisc library to explicitly list the include
path to OpenSSL and link with -lcrypto. By hiding the specific
implementation into the private namespace, we no longer enforce this.
In the long run, this might also allow us to switch cryptographic
library implementation without affecting the downstream users.
While making the isc_md_type_t type opaque, the API using the data type
was changed to use the pointer to isc_md_type_t instead of using the
type directly.
This new option was added to fill a gap in RPZ configuration
options.
It was possible to instruct BIND wheter NSIP rewritting rules would
apply or not, as long as the required data was already in cache or not,
respectively, by means of the option nsip-wait-recurse.
A value of yes (default) could incur a little processing cost, since
BIND would need to recurse to find NS addresses in case they were not in
the cache.
This behavior could be changed by setting nsip-wait-recurse value to no,
in which case BIND would promptly return some error code if the NS IP addresses
data were not in cache, then BIND would start a recursive query
in background, so future similar requests would have the required data
(NS IPs) in cache, allowing BIND to apply NSIP rules accordingly.
A similar feature wasn't available for NSDNAME triggers, so this commit
adds the option nsdname-wait-recurse to fill this gap, as it was
expected by couple BIND users.
Michał Kępień [Mon, 16 Mar 2020 10:32:46 +0000 (11:32 +0100)]
Move FreeBSD CI jobs to libvirt-based executors
To get rid of the currently used FreeBSD-specific executor, move FreeBSD
CI jobs to libvirt-based executors. Make the necessary tag and variable
adjustments.
Mark Andrews [Fri, 13 Mar 2020 00:23:40 +0000 (11:23 +1100)]
wait for the reply message before checking to avoid false negative.
Waiting for the reply message will ensure that all messages being
looked for exist in the logs at the time of checking. When the
test was only waiting for the send message there was a race between
grep and the ns1 instance of named logging that it had seen the
request.
Mark Andrews [Mon, 9 Mar 2020 00:33:05 +0000 (11:33 +1100)]
Silence missing unlock from Coverity.
Save 'i' to 'locknum' and use that rather than using
'header->node->locknum' when performing the deferred
unlock as 'header->node->locknum' can theoretically be
different to 'i'.
Ondřej Surý [Thu, 12 Mar 2020 08:45:58 +0000 (09:45 +0100)]
Stop leaking OpenSSL types and defines in the isc/hmac.h
The <isc/md.h> header directly included <openssl/hmac.h> header which
enforced all users of the libisc library to explicitly list the include
path to OpenSSL and link with -lcrypto. By hiding the specific
implementation into the private namespace, we no longer enforce this.
In the long run, this might also allow us to switch cryptographic
library implementation without affecting the downstream users.
Ondřej Surý [Wed, 11 Mar 2020 20:23:17 +0000 (21:23 +0100)]
Stop leaking OpenSSL types and defines in the isc/safe.h
The two "functions" that isc/safe.h declared before were actually simple
defines to matching OpenSSL functions. The downside of the approach was
enforcing all users of the libisc library to explicitly list the include
path to OpenSSL and link with -lcrypto. By hiding the specific
implementation into the private namespace changing the defines into
simple functions, we no longer enforce this. In the long run, this
might also allow us to switch cryptographic library implementation
without affecting the downstream users.