Bug 243234

This is probably because the sscanf implementation has a pointer that is being jumped from argument to argument which means that it appears to move outside the bounds of the object it is originally pointing at.

I've seen similar problems in other formatting functions of this type but it's not clear there's any way to solve it other than adding a suppression - the whole modus operandi of ptrcheck is to complain if a variable move outside the object it points at.

I have not really looked at the sscanf implementation (and my doctor says I should not as it won't be good for my heart), but if the pointer jumps from one arg to the other, shouldn't ptrcheck somehow get that it has been re-initialized from a new input parameter then? I would understand perfectly the message if this internal pointer somehow managed to derive from buf0 a ptr that is then suddenly pointing into buf1, buf if that ptr is really *set* to point to buf1 -- the parameter to sscanf -- shouldn't ptrcheck see that as not being derived from the previos buf0 pointer, but set to something from "outside"?

if an implementation would do 

int x = param2 - param1;
char* ptr = param1;
/* use ptr */
ptr += x;
/* use it again*/

then I would say the complaint is correct, buf if it does

char* ptr = param1;
/* use ptr */
char* ptr = param2;

imho ptrcheck should be able to detect it.


Now that I think of it, the almost 100 suppressions that I wrote for our software seem to be of a similar kind. We have a ptr variable that is always re-used for pointing to an element determine within the looping.

It would really be nice if there could be a fix for this since it is a really tedious thing to always write suppressions, since for every new case you have to determine whether it is not maybe a bug, and with our old legacy code here we find quite some bugs with ptrcheck that no other tool shows us...

Tom's analysis is right.  To make it a bit more concrete though:

You are doing 

  char buf0[..];
  char buf1[..];
  fscanf("%s %s", buf0, buf1);

Ptrcheck assumes that any insn that reads or writes a stack or
global array once will continue to read or write in that same
array, until the stack frame of the function exits.  So what you
have here is a call to _IO_vfscanf, and somewhere in that fn is a loop
that copies data to destination arrays in response to %s in the
format string.  Since you have two %s here, first it copies data
to buf0 (so Ptrcheck assumes that is the intended destination).
Then, it switches to writing buf1, and so it complains.

Of course when _IO_vfscanf exits, then Ptrcheck "forgets" everything
it knows about the instructions in _IO_vfscanf.  It has to do that so
that later calls to _IO_vfscanf are not checked against the buffers
that earlier calls wrote to.

(In reply to comment #2)
> I have not really looked at the sscanf implementation (and my doctor says I
> should not as it won't be good for my heart), but if the pointer jumps from one
> arg to the other, shouldn't ptrcheck somehow get that it has been
> re-initialized from a new input parameter then?

The problem is ..

The basic assumption is: if a memory referencing insn accesses a stack
or global array, then it will continue to access that same array until
the containing stack frame exits.  See

http://www.valgrind.org/docs/manual/pc-manual.html#pc-manual.how-works.sg-checks

So .. how, in this case, can it know that the insn which copies data
into %s destinations is switching between destination arrays?  I don't
see a way to do this.


> Now that I think of it, the almost 100 suppressions that I wrote for our
> software seem to be of a similar kind. We have a ptr variable that is always
> re-used for pointing to an element determine within the looping.

Yes.  See bullet point 5 of
http://www.valgrind.org/docs/manual/pc-manual.html#pc-manual.limitations


> It would really be nice if there could be a fix for this since it is a really
> tedious thing to always write suppressions, since for every new case you have
> to determine whether it is not maybe a bug, and

Maybe it could be improved with better heuristics, but I couldn't
think of any during development.  If you have any ideas ...


> with our old legacy code here
> we find quite some bugs with ptrcheck that no other tool shows us...

Wow.  This is the first time I heard that Ptrcheck finds real bugs.
That's great.

Are these from the stack/global array checking only?  I have
(uncommitted) a cut-down version which just does stack/global checks,
and omits the heap checks, since Memcheck does those faster.

In fact, looking at the stack/global array checking code, I think I
did a pretty stupid implementation.  It could be redone to be much
faster, but until now I never put much effort into improving it
because I assumed the tool as a whole is not very useful (didn't find
many bugs in the testing I did.)

Closing this as WONTFIX.  I actually mean CANTFIX -- I have no idea
how to, as per comment #4.  Suggestions welcomed.

Similar to the write warning in sscanf with "%s %s" as format, sprintf will cause a read warning on the same format with the same char arrays, e.g.

==4995== Invalid read of size 1
==4995==    at 0x52A9040: vfprintf (vfprintf.c:1655)
==4995==    by 0x52CDAA3: vsprintf (iovsprintf.c:42)
==4995==    by 0x52B01D6: sprintf (sprintf.c:32)
==4995==    by 0x4194A8: xf_do_job (xphireengine.c:1911)
==4995==    by 0x4119C6: xf_main (xphireengine.c:642)
==4995==    by 0x4110FB: main (xphireengine.c:490)
==4995==  Address 0xffeffd800 expected vs actual:
==4995==  Expected: stack array "tmsg" of size 80 in frame 3 back from here
==4995==  Actual:   stack array "t2msg" of size 80 in frame 3 back from here
==4995==  Actual:   is 0 after Expected

(This comment is added in the hope that it can help users find false positives from sgcheck).