> On 22/03/2017, at 11:31 PM, Mihail Groza <unlupdest...@gmail.com> wrote:
> 
> Hi Richard,
> 
> Thanks for your feedback. Let's take each issue one at a time.
> 
> 0) Bison version
>  You're right, grammar was modified to use some features that aren't
>  present in older versions of Bison.

There are three issues here.

(1) Another of my machines has the 1.875 (2002) release of Bison,
    which let me verify that bison has had the -p (or
    --name-prefix=) command-line option for a very long time.
    So 15 years ago it was *supposed* to be easy to have
    multiple Bison-generated parsers in a program without any
    special cruft inside the .y file or any mangling of the output.

    The -p option is arguably better because the *same* grammar
    can be used with *different* prefixes in different programs.

    For that matter, the -p option is compatible with the 1996
    version of yacc on the same machine (which is System V Yacc,
    not Bison in disguise).  The -p option is required by the
    Single Unix Specification version 4 shell and utilities.

    So it appears that there was a practical alternative.

(2) There's nothing actually *wrong* with using recent features
    of tools.  If it sounded as though I was complaining about
    you using new features, I am sorry.

(3) The problem is that the installation instructions don't SAY
    which version(s) of Bison are needed and the build process
    doesn't CHECK.  I had to find out the hard way.
> 
> 
> 2) test failure for system header sys/types.h

> Splint has a
>  somewhat surprisingly behaviour, meaning that by default, unless is a
>  blocker (as in a parsing error), all errors in system headers are
>  ignored and not reported, thus you might not see any complaint about
>  failing to find/include a system header. This is what I suspect
>  happens. To test that indeed this is the problem, run
>    $(SPLINT) -nof -hints -booltype "bool" utype.c
>  (the equivalent of the test) and look for the parsing error. Assuming
>  that it still exists, now run
>    $(SPLINT) -nof -hints -booltype "bool" utype.c -keep
>  which does the same thing, but additionally instructs Splint to keep
>  temporary preprocessed files (the name of the file will be printed).
>  Now look into the temporary (preprocessed) file, and look for the
>  missing definition ("__int64_t" in this case). If it's not there,
>  either the file it declared it wasn't included (preprocessor line
>  comments should help you find if that file was indeed included), or
>  you've found (yet another) bug.
> 
>  Now, if the file wasn't included, most likely it is in a folder that
>  Splint doesn't know of (and thus doesn't look into). Using -sysdirs
>  flag to change the default include path should be a temporary fix,
>  once you know your system's correct list of include paths, should
>  reconfigure/recompile Splint to make the solution permanent.
> 
>  Alternatively, if the header containing the definition is included,
>  I'll need your assistance in tracking such a bug (as I said, no OSX).

Let's try a tiny example.
m% cat foo.c
#include <unistd.h>

ssize_t (*f)(int, void const *, size_t) = (write);

m% splint foo.c
Splint 3.1.2a --- Mar 22 2017

foo.c:1: Include file <unistd.h> matches the name of a POSIX library, but the
    POSIX library is not being used.  Consider using +posixlib or
    +posixstrictlib to select the POSIX library, or -warnposix to suppress this
    message.
  Header name matches a POSIX header, but the POSIX library is not selected.
  (Use -warnposixheaders to inhibit warning)
/usr/include/sys/_types.h:55:36: Parse Error:
    Suspect missing struct or union keyword: __int64_t :
    int. (For help on parse errors, see splint -help parseerrors.)
*** Cannot continue.

The line is
typedef __int64_t       __darwin_blkcnt_t;      /* total blocks */

With -nof -hints -booltype "bool" nothing changes.

Adding -keep, the temporary file begins like this:
# 1 "/home/cshome/o/ok/Smalltalk/OCT/foo.c"
# 1 "/usr/include/unistd.h" 1
# 1 "/usr/include/_types.h" 1
# 1 "/usr/include/sys/_types.h" 1
# 1 "/usr/include/sys/cdefs.h" 1
# 153 "/usr/include/sys/cdefs.h"
# 253 "/usr/include/sys/cdefs.h"
# 285 "/usr/include/sys/cdefs.h"
# 422 "/usr/include/sys/cdefs.h"
# 442 "/usr/include/sys/cdefs.h"
# 1 "/usr/include/sys/_symbol_aliasing.h" 1
# 533 "/usr/include/sys/cdefs.h" 2
# 588 "/usr/include/sys/cdefs.h"
# 1 "/usr/include/sys/_posix_availability.h" 1
# 599 "/usr/include/sys/cdefs.h" 2
# 32 "/usr/include/sys/_types.h" 2
# 1 "/usr/include/machine/_types.h" 1
# 33 "/usr/include/sys/_types.h" 2
# 51 "/usr/include/sys/_types.h"
typedef __int64_t       __darwin_blkcnt_t;

and the last line is where splint chokes.

Now the significant thing here is that
/usr/include/machine/_types.h  IS mentioned but
/usr/include/i386/types.h   is NOT.

machine/_types.h is

<<standard apple header>>
<<double include protection>>
#if defined(__i386__) || defined(__x86_64__)
#include "i386/types.h"
#else
#error architecture not supported
#endif
<<end double include protection>>

Adding -D__x86_64__ or -D__i386__ to the splint command line
fixes this problem.
> 
> 3) the *_unlocked I/O functions.

Thank you very much.

>  In relation to you making clean and then rebuilding the libraries but
>  still failing to see the changed behaviour, Splint doesn't read the
>  library from the build directory (unless instructed to do so using
>  LARCH_PATH), by default it reads from its installation directory. Thus,
>  if you make further changes to the library that you want to test,
>  either change LARCH_PATH environment variable to the path used for
>  building, or actually install the new libraries
>    (make -C lib/ install)

Peccavi.
> 
> 4) (void)0 & -noeffect
>  I don't understand what doesn't work. Please provide a small sample
>  for me.

m% cat foo.c
/*1*/ int main(void) {
/*2*/    (void)0;
/*3*/    1;
/*4*/    return 0;
/*5*/ };
m% cc -Wall -Wextra -c foo.c
foo.c:3:10: warning: expression result unused [-Wunused-value]
/*3*/    1;
         ^
1 warning generated.

m% splint foo.c
Splint 3.1.2a --- Mar 22 2017

foo.c: (in function main)
foo.c:2:10: Statement has no effect: (void)0
  Statement has no visible effect --- no values are modified. (Use -noeffect to
  inhibit warning)
foo.c:3:10: Statement has no effect: 1

Finished checking --- 2 code warnings

I DON'T want a warning on line /*2*/.
I DO want a warning on line /*3*/.

The default behaviour is to warn about BOTH lines.
With -noeffect splint gives NO warnings.

There are plenty of places where you need to say
"nothing should happen here".  For example,
#ifdef  LOGGING
#define LOG(format,...) (void)fprintf(stderr, format __VAR_ARGS__)
#else
#define LOG(format,...) (void)0
#endif

If you use other static checkers, like lint,
    if (...) {
    } else
    if (...) {
        ...
    } else {
        ...
    }
produces warnings like
(4) warning: statement has no consequent: if
To prevent this bogus warning, you whack in (void)0;
and all's bowmon.  (Did anyone but Jeffrey Farnol ever
use that word?  I can't find it in the OED!)

----------------------------------------------

EMPTY MACRO PARAMETERS (new issue)

#define foo() 1
#define bar(x) 2

int test[] = {
    foo(),
    bar()
};

In C89, the definition of foo was illegal and both macro
calls were illegal.

In C99, the file is completely legal.   The grammar explicitly
allows the declaration of a macro with an empty formal parameter
list, and section 6.10.3 paragraph 4 says:
"the number of arguments (including those arguments consisting of no 
preprocessing tokens) shall equal the number of parameters".
C11 has not reversed this change.  The same changes were made to
the preprocessor in C++.

m% splint foo.c
Splint 3.1.2a --- Mar 22 2017

foo.c:6:10: macro `bar' used without args
  Preprocessing error. (Use -preproc to inhibit warning)
Preprocessing error for file: /home/cshome/o/ok/Desktop/Sensors/foo.c
*** Cannot continue.

It might perhaps make sense for splint to have
--std=c89 --std=c99 --std=c11 options, but at least the
default behaviour should be to allow what c99 allows.

-------------------------------------------

BOGUS VARIABLE REDEFINITION WARNINGS (new issue)
BAD ADVICE IN VARIABLE REDEFINITION WARNINGS (new issue)

I need to have a bunch of records initialised to point
to each other.

struct Linked { struct Linked *x; };
static struct Linked a;
static struct Linked b;
static struct Linked a = {&b};
static struct Linked b = {&a};

m% splint foo.c
Splint 3.1.2a --- Mar 22 2017

foo.c:4:22: Variable a redefined
  A function or variable is redefined. One of the declarations should use
  extern. (Use -redef to inhibit warning)
   foo.c:2:22: Previous definition of a
foo.c:4:27: Immediate address &b used as initial value for implicitly only:
               a.x = &b
  An immediate address (result of & operator) is transferred inconsistently.
  (Use -immediatetrans to inhibit warning)
foo.c:5:22: Variable b redefined
   foo.c:3:22: Previous definition of b
foo.c:5:27: Immediate address &a used as initial value for implicitly only:
               b.x = &a

Finished checking --- 4 code warnings

The last two warnings are to do with splint's pointer tracking,
and I'm happy to shut that up or annotate.  (The real code is
generated from a data file, so it's a small matter of changing
the generator.)  The problem I have is with the first two
warnings.

The variables are *NOT* "redefined".  We have "tentative
definitions" (think of them as forward definitions for data)
to start with, followed by initialised declarations which
are the real definitions.  To add insult to injury, the
advice in the warning is DISASTROUSLY WRONG.  These are
and must be static declarations.  Making one of them static
and the other extern would nowadays be a serious error.
Using 'extern' instead of 'static' for the first two
variable declarations and deleting 'static' from the second
two would be legal, BUT it would export the identifiers to
the world, which it is important not to do.

Section 6.9.2 of C99 says

"[para 2] A declaration of an identifier for an object that
 has file scope without an initializer, and without a
 storage-class specifier or with the storage-class specifier
 static, constitutes a tentative definition.
 If a translation unit contains one or more tentative
 definitions for an identifier, and the translation unit
 contains no external definition for that identifier,
 then the behavior is exactly as if the translation unit
 contains a file scope declaration of that identifier,
 with the composite type as of the end of the translation
 unit, with an initializer equal to 0.

 [para 3] If the declaration of an identifier for an
 object is a tentative definition and has internal linkage,
 the declared type shall not be an incomplete type.
"

The problem with using -redef to force splint to accept
legal, meaningful, and useful C is that it also forces
splint to accept stuff that ISN'T.

Consider the example

static int a[2];
static int a[3];

int *p = &a[0];

This is a case where a variable is redefined *with a different
type*.  This is beyond doubt or peradventure an actual error.
This is not legal C and never has been.  But with -redef,
splint is perfectly happy.  Without -redef, splint gives
advice that is wrong:

foo.c:2:12: Variable a redefined
  A function or variable is redefined. One of the declarations should use
  extern. (Use -redef to inhibit warning)
   foo.c:1:12: Previous definition of a

It's wrong in three ways.
Consider
extern int a[2];
static int a[3];

(1) This is not legal C.  Splint has recommended a change
    that does not make the program legal.
(2) If it were legal, we'd now be exporting a variable we
    wanted NOT to export (otherwise it wouldn't be static).
(3) The change doesn't even shut splint up itself.  We now
    get
foo.c:2:12: Variable a shadows outer declaration
  An outer declaration is shadowed by the local declaration. (Use -shadow to
  inhibit warning)
   foo.c:1:12: Previous declaration of a: int [2]

which is especially confusing, because the two declarations are
at the same level.  Sigh.

A top-level variable must be *initialised* at most once
but may be *declared* any number of times as long as all
the declarations are either the same type or one set has
an incomplete type and the other set completes that type.

----------------------

YOU DO NOT HAVE UNLIMITED TIME AND HAVE ALREADY GIVEN MUCH

It would be unreasonable of me to expect you to fix every problem
in splint.  You've done *way* more for splint than I ever have
already.  I'm mentioning these issues so that they can be
logged as known bugs.



_______________________________________________
splint-discuss mailing list
splint-discuss@mail.cs.virginia.edu
http://www.cs.virginia.edu/mailman/listinfo/splint-discuss

Reply via email to