userspace assert/printf issue

[Barret Rhoden]

while debugging something, an issue with printf and vcore context
popped up.

the printf family of calls is dangerous from vcore context. if an
output stream is switched to buffered, which apps can do, then printf
calls that use glibc's printf will run off the end of vcore context's
stack. (check out buffered_vfprintf() in
glibc-2.19/stdio-common/vfprintf.c).

this problem also presents itself when an assert fails in VC ctx,
since that triggers a printf.

option 1: a temporary workaround for asserts is to use parlib/rassert.h,
which overrides assert() and uses our debug printf. i've got a few
patches that do that (and rename it to assert.h). parlib's assert also
does a few nicer things, so i like our assert more than glibc's in
general.

likewise, we can do something similar with stdio.h for
printf/fprintf/etc. (rename parlib/ros_debug.h -> parlib/stdio.h,
override printf/fprintf, etc).

the downside is that any code that might call assert/printf from vcore
context will need to #include a parlib header, e.g. parlib/assert.h
instead of the usual assert.h. the way we find out that we failed to
do this is when a process blows up on a printf, most all of which are
for debugging. this also includes tests and applications that happen to
use vcore context things, like an event handler debug print.

option 2: one way around this would be to replace glibc's assert.h with
our own, though that would probably have far reaching consequences,
especially since parlib/assert.h will include parlib/stdio.h. having
every program use our cheap printfs doesn't seem like a great approach.

for the most part, the only files that #include any form of assert are
.c files, not headers, so there's no real 'taint' spreading of
parlib/assert.h yet.

option 3: a middle-ground option between forcing #include
parlib/assert.h and replacing glibc's assert.h would be to have
parlib/parlib.h #include parlib/assert.h, so any library or program
that includes that would get parlib's asserts/printfs. the assumption
there is that if you're doing something with vcore context, then you
probably included that. or perhaps we put the #include in vcore.h.

incidentally, parlib.h already includes parlib/ros_debug.h, but that
doesn't override printf yet - it's still using ros_debug() for printing.

any thoughts one way or the other? the easiest path, with the least
impact on apps trying to build against glibc, might be to just use
parlib/assert.h and parlib/stdio.h on our own (option 1), and be careful
with code that touches vcore context.

barret

[Kevin Klues]

We could have assert.h #include parlib/assert.h and then
check__vcore_context to call ros_debug instead of printf.

> --
> You received this message because you are subscribed to the Google Groups "Akaros" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to akaros+un...@googlegroups.com.
> To post to this group, send email to aka...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.



--
~Kevin

[Kevin Klues]

Sorry, I wrote that really fast.

I meant we could modify glibc's assert() to branch based on the value
of __vcore_context to either user printf or ros_debug() depending on
its value.

--
~Kevin

[Barret Rhoden]

On 2015-10-21 at 15:59 Kevin Klues <klu...@gmail.com> wrote:
> Sorry, I wrote that really fast.
>
> I meant we could modify glibc's assert() to branch based on the value
> of __vcore_context to either user printf or ros_debug() depending on
> its value.

That might have some issues. In part, glibc's assert.h probably needs
to work for all of glibc, so having parts of parlib available might not
work.

The other part is that it's not just assert - it's any printf. For
instance, the fprintfs in parlib signal.c also trigger PFs. When I
fixed d95390043f8c ("Check for ctx in default_core_handler()") I had to
manually change those fprintfs to ros_debug calls.

Unfortunately, overriding these parts of glibc is a little nastier than
the weak aliased functions.

Barret

[Barret Rhoden]

On 2015-10-21 at 15:59 Kevin Klues <klu...@gmail.com> wrote:

> Sorry, I wrote that really fast.
>
> I meant we could modify glibc's assert() to branch based on the value
> of __vcore_context to either user printf or ros_debug() depending on
> its value.

I ended up doing something similar to this. I kept our own assert for
any code that includes parlib headers - I like being able to drop a
breakpoint in the assert error case.

For printf, I overrode printf to call our own, which checks
in_vcore_context, but only printf is overriden. The others, e.g.
vprintf, are still glibcs. Our printf then calls either ros_vdebug or
vprintf. The only other tidbit that is missing is that we have some
printf specifiers used in glibc that I didn't put in ros_debug.

Barret

[Kevin Klues]

Sounds good to me.