[PATCH v2 2/2] gdbserver/tracepoint.cc: use snprintf in gdb_agent_socket_init

[Pedro Alves]

On 2021-12-10 21:04, Simon Marchi via Gdb-patches wrote:
> If we modify tracepoint.cc to try to use a too long unix socket name,
> for example by modifying SOCK_DIR to be:
> 
>     #define SOCK_DIR "/tmp/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut"
> 
> ... trying to start an application with libinproctrace.so loaded
> crashes:
> 
>     $ LD_PRELOAD=/usr/lib/x86_64-linux-gnu/libasan.so.6:./libinproctrace.so /bin/ls
>     /home/smarchi/src/binutils-gdb/gdbserver/../gdbsupport/common-utils.cc:69: A problem internal to GDBserver in-process agent has been detected.
>     xsnprintf: Assertion `ret < size' failed.
> 
> Looking at the rest of the socket initialization code, the intent seems
> to be that if something goes wrong, we warn but let the program
> execute.  So crashing on this failed assertions seems against the intent.

Yes, that's the intent.

> 
> Commit 6cebaf6e1ae4 ("use xsnprintf instead of snprintf.") changed this
> code to use xsnprintf instead of snprintf, introducing this assertion.
> Before that, snprintf would return a value bigger that UNIX_PATH_MAX and
> the "if" after would catch it and emit a warning, which is exactly what
> we want.  That change was done because LynxOS didn't have snprintf.
> Since LynxOS isn't supported anymore, we can simply revert to use
> snprintf there.
> 
> With this patch, we get a warning (printed by the caller of
> gdb_agent_socket_init), but the prorgam keeps executing:

prorgam -> program

> 
>     $ LD_PRELOAD=/usr/lib/x86_64-linux-gnu/libasan.so.6:./libinproctrace.so /bin/ls
>     ipa: could not create sync socket
>     ...
> 

Both patches LGTM.