Re: [PATCH v3 9/9] compile: compile printf: gdbserver support

[Pedro Alves <palves@redhat.com>]

On 04/26/2015 10:33 AM, Jan Kratochvil wrote:
> On Sat, 11 Apr 2015 21:44:37 +0200, Jan Kratochvil wrote:
>> former patch injects plain:
>> 	printf (...);
>> This patch injects gdbserver-compatible:
>> 	f = open_memstream (&s, ...);
>> 	fprintf (f, ...);
>> 	fclose (f);
>> 	return s;
> 
> I have realized this print+printf patchset introduces calling inferior
> implicit malloc() + explicit free() (by free_inferior_memory) which the
> original 'compile code' series avoided (using gdbarch_infcall_mmap() instead).
> The goal was not to crash the inferior futher with print commands when
> analyzing corrupted inferior memory lists.

Right.  The "compile code" infrastructure should restrict itself
to async-signal-safe functions for its internal mechanisms for that reason.
Of course, if the expression the user injects runs non-async-signal-safe
at the wrong time, the user gets what she asked for.

> 
> I somehow expected that printf()/fprintf() are so heavyweight they will call
> malloc() on their own so this mmap goal is no longer achievable for printf.
> But I have found now glibc in most real world cases uses just alloca().
> 
> The problem is even calling fmemopen() instead of open_memstream() still
> implicitly calls malloc() - for fmemopen_cookie_t and for FILE.
> 
> The only idea I have is to redirect by a breakpoint glibc's implicit calls to
> malloc() into GDB's allocator by inferior mmap.  But that seems a bit ugly.

Using mmap along with snprintf would be safer, but given that snprintf is
not async-signal-safe in general either, it's fine with me to leave this
as you have it.

I think the manual should say that the command internally may call
functions that are not async-signal-safe though.

> So currently keeping it as a known bug.

Otherwise looks good to me.

Thanks,
Pedro Alves