[src/trunk]: src/usr.bin/xlint/lint1 lint: demonstrate wrong message 204 (sin...

[rillig <rillig%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/4e4ea1a638f0
branches:  trunk
changeset: 958498:4e4ea1a638f0
user:      rillig <rillig%NetBSD.org@localhost>
date:      Fri Jan 08 01:40:03 2021 +0000

description:
lint: demonstrate wrong message 204 (since 2020-12-31)

In func.c 1.39 from 2020-12-31 18:51:28, the check that controlling
expressions are indeed scalar was extended from while and for loops to
if statements as well.  It just seemed to have been an oversight.

This revealed a bug in lint, which didn't accept the following valid
code snippet from lib/libpthread/pthread.c:634:

        void _malloc_thread_cleanup(void) __weak;
        ...
        if (_malloc_thread_cleanup)
                _malloc_thread_cleanup();

Testing a function (instead of a function pointer) for truthiness is
probably rare since most functions are defined unconditionally.  For
weak functions it comes in handy though.

Clang-Tidy suggests to prefix the function with '&' to silence its
warning.  Doing that revealed a non-obvious behavior in build_ampersand,
which does not add the AMPER node to the expression even though it is
clearly mentioned in the code.  That is left for further research.

Once the original bug is fixed, it probably doesn't matter whether the
AMPER is discarded or retained since check_controlling_expression would
add it back.  There's probably a reason though to sometimes discard the
AMPER and sometimes retain it.

diffstat:

 tests/usr.bin/xlint/lint1/msg_204.c   |  33 ++++++++++++++++++++++++++++++---
 tests/usr.bin/xlint/lint1/msg_204.exp |   3 ++-
 usr.bin/xlint/lint1/func.c            |   6 ++++--
 3 files changed, 36 insertions(+), 6 deletions(-)

diffs (75 lines):

diff -r 240b99b49db3 -r 4e4ea1a638f0 tests/usr.bin/xlint/lint1/msg_204.c
--- a/tests/usr.bin/xlint/lint1/msg_204.c       Fri Jan 08 01:17:55 2021 +0000
+++ b/tests/usr.bin/xlint/lint1/msg_204.c       Fri Jan 08 01:40:03 2021 +0000
@@ -1,7 +1,34 @@
-/*     $NetBSD: msg_204.c,v 1.1 2021/01/02 10:22:44 rillig Exp $       */
+/*     $NetBSD: msg_204.c,v 1.2 2021/01/08 01:40:03 rillig Exp $       */
 # 3 "msg_204.c"
 
 // Test for message: controlling expressions must have scalar type [204]
 
-TODO: "Add example code that triggers the above message."
-TODO: "Add example code that almost triggers the above message."
+extern void
+extern_function(void);
+
+void (*function_pointer)(void);
+
+/*
+ * Since func.c 1.39 from 2020-12-31 18:51:28, lint had produced an error
+ * when a controlling expression was a function.  Pointers to functions were
+ * ok though.
+ */
+void
+bug_between_2020_12_31_and_2021_01_08(void)
+{
+       if (extern_function)
+               extern_function();
+
+       /*
+        * FIXME: For some reason, the ampersand is discarded in
+        *  build_ampersand.  This only has a visible effect if the
+        *  t_spec in check_controlling_expression is evaluated too early,
+        *  as has been the case before func.c 1.52 from 2021-01-08.
+        */
+       if (&extern_function)
+               extern_function();
+
+       /* This one has always been ok since pointers are scalar types. */
+       if (function_pointer)
+               function_pointer();
+}
diff -r 240b99b49db3 -r 4e4ea1a638f0 tests/usr.bin/xlint/lint1/msg_204.exp
--- a/tests/usr.bin/xlint/lint1/msg_204.exp     Fri Jan 08 01:17:55 2021 +0000
+++ b/tests/usr.bin/xlint/lint1/msg_204.exp     Fri Jan 08 01:40:03 2021 +0000
@@ -1,1 +1,2 @@
-msg_204.c(6): syntax error ':' [249]
+msg_204.c(19): controlling expressions must have scalar type [204]
+msg_204.c(28): controlling expressions must have scalar type [204]
diff -r 240b99b49db3 -r 4e4ea1a638f0 usr.bin/xlint/lint1/func.c
--- a/usr.bin/xlint/lint1/func.c        Fri Jan 08 01:17:55 2021 +0000
+++ b/usr.bin/xlint/lint1/func.c        Fri Jan 08 01:40:03 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: func.c,v 1.51 2021/01/05 00:22:04 rillig Exp $ */
+/*     $NetBSD: func.c,v 1.52 2021/01/08 01:40:03 rillig Exp $ */
 
 /*
  * Copyright (c) 1994, 1995 Jochen Pohl
@@ -37,7 +37,7 @@
 
 #include <sys/cdefs.h>
 #if defined(__RCSID) && !defined(lint)
-__RCSID("$NetBSD: func.c,v 1.51 2021/01/05 00:22:04 rillig Exp $");
+__RCSID("$NetBSD: func.c,v 1.52 2021/01/08 01:40:03 rillig Exp $");
 #endif
 
 #include <stdlib.h>
@@ -537,6 +537,8 @@
 static tnode_t *
 check_controlling_expression(tnode_t *tn)
 {
+       // FIXME: dereference before null check
+       // FIXME: This is evaluated too early; see test msg_204
        tspec_t t = tn->tn_type->t_tspec;
 
        if (tn != NULL)