snprintf() doesn't use format size specifiers

[Norman Feske]

Hi Pirmin,

On 04/21/2015 07:39 PM, Duss Pirmin wrote:
> I can share the code, where I had the problem, but it is not what I  
> would call a small example, but it's also not that big.
> 
> It is in an older version of my TraceTool.
> 
> https://github.com/trimpim/TraceTool/blob/2f282a9bda1c32ed21019e1218ea6e3226d04064/src/TraceTool/SubjectList.cpp
> 
> In the method SubjectList::toString() the data from the list of  
> subjects is stored in to a string buffer, that is later sent to a  
> terminal session.

thank you for sharing the code. I could indeed reproduce a segmentation
fault (I ran the program on Linux). I could observe that the symptom
such as the fault address changed when I modified the program. For
example, by not linking the libc, removing the call to Genode::snprintf,
or moving a constructor implementation to a header file. This erratic
change of symptoms generally hints at some kind of memory corruption. I
successively simplified the program further and further while making
sure that the problem persisted. In the end, I arrived at the situation
[1]. This final program has actually nothing in common with your
original program any longer but the kind of crash produced by it remains
the same. With the simple test program, I could spot the bug.

The issue is apparently the C++11 list-style initialization of a
reference member. In the constructor of the 'TerminalClient', the
mrSubjectList member is initialized via the following construct:

  mrSubjectList { rSubjectList }

This way, the compiler creates a temporary copy of the SubjectList and
initializes the reference member with the temporary copy. This is of
course not what you want because once the temporary copy disappears, the
TerminalClient will keep working with a dangling reference. Instead, I
presume that you intended to simply forward the reference. When using a
normal C++ initializer, the program works as intended:

  mrSubjectList ( rSubjectList )

As a precaution for issues like this, it may be helpful to disallow the
copying of the SubjectList by letting the class inherit the
Genode::Noncopyable class as found in [2]. This way, the compiler will
detect such unintended copies.

[1]
https://github.com/nfeske/genode/commit/1d78f58cb54d88c6d5da1290bb7449f0a1705560
[2]
https://github.com/genodelabs/genode/blob/master/repos/base/include/util/noncopyable.h

Cheers
Norman

-- 
Dr.-Ing. Norman Feske
Genode Labs

http://www.genode-labs.com · http://genode.org

Genode Labs GmbH · Amtsgericht Dresden · HRB 28424 · Sitz Dresden
Geschäftsführer: Dr.-Ing. Norman Feske, Christian Helmuth