Re: multiple source files [was four sparse patches]

Re: multiple source files [was four sparse patches]

[Geoff Johnstone]

> > I've attached four patches that I've written for sparse to use it
> > for a userland project.
>
>  Regarding incomplete structs, your patch seems reasonable as far as
>  I know, and it doesn't break the test suite, so I've applied and
>  pushed it.  Per your concerns, if this patch doesn't represent the
>  correct fix, the code can change later when we have a test case
>  that breaks with this patch.  Please do consider writing a patch
>  for a new test case based on your example.

OK - I sat down to write a test case and had trouble reproducing
because it turns out that the problem has a different cause.
Take three files:

/* test.h */
typedef struct foo *Foo;

void a (Foo foo);
void b (Foo foo);


/* test1.c */
#include "test.h"
void a (Foo foo) { }


/* test2.c */
#include "test.h"
void b (Foo foo) { }


With sparse 0.4.1:

$ sparse test1.c
$ sparse test2.c
$ sparse test[12].c
test2.c:3:6: error: symbol 'b' redeclared with different type
(originally declared at test.h:4) - incompatible argument 1 (different
base types)

...i.e. the problem only occurs with multiple source files on the
sparse command line. (The userland project mentioned above has a build
system that invokes the compiler once, passing all necessary source
files, because it also builds on Windows, where process creation is
remarkably slow so invoking cl.exe once per source file is prohibitive).

I think that the problem may be rooted in main():
  // Expand, linearize and show it.
  check_symbols(sparse_initialize(argc, argv, &filelist));
  FOR_EACH_PTR_NOTAG(filelist, file) {
    check_symbols(sparse(file));
  } END_FOR_EACH_PTR_NOTAG(file);

i.e. sparse doesn't reinitialize for each source file.

My patch a few weeks ago silences the error, but in the light of this
I think that it's working around rather than fixing the problem. The
"correct" fix would seem to be to re-initialise sparse for each source
file. However, sparse's data structures weren't written with this in
mind, so a patch would be rather invasive. It might be simpler to say
that sparse must only be invoked on one source file and to modify cgcc
instead?

Re: multiple source files [was four sparse patches]

[Josh Triplett]

On Fri, 2008-05-02 at 19:58 +0100, Geoff Johnstone wrote:
> Take three files:
> 
> /* test.h */
> typedef struct foo *Foo;
> 
> void a (Foo foo);
> void b (Foo foo);
> 
> 
> /* test1.c */
> #include "test.h"
> void a (Foo foo) { }
> 
> 
> /* test2.c */
> #include "test.h"
> void b (Foo foo) { }
> 
> 
> With sparse 0.4.1:
> 
> $ sparse test1.c
> $ sparse test2.c
> $ sparse test[12].c
> test2.c:3:6: error: symbol 'b' redeclared with different type
> (originally declared at test.h:4) - incompatible argument 1 (different
> base types)
> 
> ...i.e. the problem only occurs with multiple source files on the
> sparse command line. (The userland project mentioned above has a build
> system that invokes the compiler once, passing all necessary source
> files, because it also builds on Windows, where process creation is
> remarkably slow so invoking cl.exe once per source file is prohibitive).

I've observed this problem as well, and I would like to see it fixed.
Many other legitimate reasons exist to run the compiler on many files at
once.  For example, consider GCC's --combine and -fwhole-program
options.

> I think that the problem may be rooted in main():
>   // Expand, linearize and show it.
>   check_symbols(sparse_initialize(argc, argv, &filelist));
>   FOR_EACH_PTR_NOTAG(filelist, file) {
>     check_symbols(sparse(file));
>   } END_FOR_EACH_PTR_NOTAG(file);
> 
> i.e. sparse doesn't reinitialize for each source file.
> 
> My patch a few weeks ago silences the error, but in the light of this
> I think that it's working around rather than fixing the problem. The
> "correct" fix would seem to be to re-initialise sparse for each source
> file. However, sparse's data structures weren't written with this in
> mind, so a patch would be rather invasive. It might be simpler to say
> that sparse must only be invoked on one source file and to modify cgcc
> instead?

I'd like Sparse to handle multiple files on the same command line.  I
don't mind invasive patches, though as always I'd like to see them
broken up into incremental patches when possible.

I don't necessarily want Sparse fully re-initialized to the point where
it forgets what it learned from the previous source files.  Remembering
the contents of previous files can only help with cross-module analysis.
However, Sparse needs to isolate symbols by file, and avoid carrying
over the visible symbols from one file to the next.

- Josh Triplett

Re: multiple source files [was four sparse patches]

[Sam Ravnborg]

> I'd like Sparse to handle multiple files on the same command line.  I
> don't mind invasive patches, though as always I'd like to see them
> broken up into incremental patches when possible.

I once made a test patch for kbuild that allowed me to run
sparse on all files for a module.
It started out nicely but when I hit the xfs source my machine went
in swap death.
So there is a memory consumption issue to take care of too.

	Sam