Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLion debugger passes spurious arguments with gdb 10.2 #2958

Closed
DCNick3 opened this issue Sep 27, 2021 · 13 comments
Closed

CLion debugger passes spurious arguments with gdb 10.2 #2958

DCNick3 opened this issue Sep 27, 2021 · 13 comments
Assignees
Labels
awaiting-maintainer Awaiting review from Bazel team on issues lang: c++ C++ rules integration product: CLion CLion plugin topic: debugging Issues related to debugging type: bug

Comments

@DCNick3
Copy link

DCNick3 commented Sep 27, 2021

The clion debugger support seems to rely on google-specific gdbserver behaviour.

It redirects stdout and stderr using shell-like syntax (>stdout 2>stderr), but this seems not to be available in mainline gdb (as compared to NDK gdb available here https://android.googlesource.com/toolchain/gdb/)

image

(for both servers clients were ran with gdb --eval-command='target remote localhost:5006' --eval-command='c')

Due to this all bazel C++ executables being debugged using CLion extension get extra command line arguments. It would be nice to have a work around for this.

Possible solutions with varying levels of adequateness I can think of:

  • allow to disable the redirection
  • allow to use custom gdbserver with instructions on how to get the required patch/provide bundled gdbserver that works
  • use some other mechanism to get the output redirected (is there a viable way?)
  • get the patch merged into mainline gdb (need to isolate the patch though)

For reference & googleability: the arguments I get are "1>/tmp/gdbserver_wrapper.KxJr27Av/inferior.out" and "2>/tmp/gdbserver_wrapper.KxJr27Av/inferior.err"

@DCNick3
Copy link
Author

DCNick3 commented Sep 27, 2021

It may have to do with startup-with-shell variable and this patch. My gdbserver actually supports the --startup-with-shell flag added along with this patch, so I have no idea why this does not work. Will continue debugging...

@DCNick3
Copy link
Author

DCNick3 commented Sep 28, 2021

It seems that it has nothing to do with google patches, as this works with (mainline) gdb 9.2, but does not work with gdb 10.2

@DCNick3
Copy link
Author

DCNick3 commented Sep 28, 2021

bisecting the gdb yielded this commit:

commit bea571ebd78ee29cb94adf648fbcda1e109e1be6
Author: Michael Weghorn <m.weghorn@posteo.de>
Date:   Mon May 25 11:39:43 2020 -0400

    Use construct_inferior_arguments which handles special chars
    
    Use the construct_inferior_arguments function instead of
    stringify_argv to construct a string from the program
    arguments in those places where that one is then passed
    to fork_inferior (linux-low, lyn-low), since
    construct_inferior_arguments properly takes care of
    special characters, while stringify_argv does not.
    Using construct_inferior_arguments seems "natural", since its
    documentation also mentions that it "does the
    same shell processing as fork_inferior".
    
    Since construct_inferior_args has been extended to do
    proper quoting for Windows shells in commit
    5d60742e2dd3c9b475dce54b56043a358751bbb8
    ("Fix quoting of special characters for the MinGW build.",
    2012-06-12), use it for the Windows case as well.
    (I could not test that case myself, though.)
    
    Adapt handling of empty args in function 'handle_v_run'
    in gdbserver/server.cc to just insert an empty string
    for an empty arg, since that one is now properly handled
    in 'construct_inferior_arguments' already (and inserting
    a "''" string in 'handle_v_run' would otherwise
    cause that one to be treated as a string literally
    containing two quote characters, which
    'construct_inferior_args' would preserve by adding
    extra escaping).
    
    This makes gdbserver properly handle program args containing special
    characters (like spaces), e.g. (example from PR25893)
    
      $ gdbserver localhost:50505 myprogram "hello world"
    
    now properly handles "hello world" as a single arg, not two separate
    ones ("hello", "world").
    
    gdbserver/ChangeLog:
    
            PR gdbserver/25893
            * linux-low.cc (linux_process_target::create_inferior),
            lynx-low.cc (lynx_process_target::create_inferior),
            win32-low.cc (win32_process_target::create_inferior): Use
            construct_inferior_arguments instead of stringify_argv
            to get string representation which properly escapes
            special characters.
            * server.cc (handle_v_run): Just pass empty program arg
            as such, since any further processing is now handled via
            construct_inferior_arguments.
    
    Change-Id: Ibf963fcd51415c948840fb463289516b3479b0c3

This changes the way the arguments are handled so it makes sense that it broke the redirection. Will probably go & report it to them...

@DCNick3 DCNick3 changed the title CLion debugger relies on stdio redirection not available in mainline gdb CLion debugger passes spurious arguments with gdb 10.2 Sep 28, 2021
@DCNick3
Copy link
Author

DCNick3 commented Sep 28, 2021

@DCNick3
Copy link
Author

DCNick3 commented Sep 28, 2021

gdb compiled with this (quite dirty) patch fixes the issue: https://gist.github.com/DCNick3/d12cb304282d226b57af1fc512c0e24f

@jesseschalken
Copy link

As an alternative to building gdb with the above patch by @DCNick3, downgrading gdb and gdbserver to 9.2 (from Ubuntu 20.10, for example) and pointing CLion at /usr/bin/gdb instead of the bundled version (which is 10.2 for CLion 2021.1.3) in the settings also fixes it.

@redsun82
Copy link

I just got hit by this. Is there any workaround available? I've seen a patch was submbitted to GDB, but I've seen this behaviour also on my GDB version 11.1. Couldn't this be fixed in the plugin, using another means of redirection?

@redsun82
Copy link

a temporary workaround I've found, is to go to the gdbserver wrapper used by the plugin (in my case, this was found on ~/.local/share/JetBrains/CLion2021.3/clwb/gdb/gdbserver), and comment out the following two lines

new_args+=("1>${inferior_stdout}")
new_args+=("2>${inferior_stderr}")

as far as I can tell, this means now the inferior output streams go together with gdserver's ones, but that does not seem to be a big problem for me.

I guess another workaround would be to add special treatment of 1>... and 2>... arguments to the application itself, but that feels wrong

@ThomasGreenhill
Copy link

Do any of you have updates on more sustainable solutions for this issue?

@sgowroji sgowroji added type: bug product: CLion CLion plugin lang: c++ C++ rules integration topic: debugging Issues related to debugging labels Oct 28, 2022
@sgowroji
Copy link
Member

sgowroji commented Nov 8, 2022

Hello @DCNick3, Are you still looking support on the issue and workaround mentioned here worked for you ?

@sgowroji sgowroji added the more-data-needed Awaiting response from issue author label Nov 8, 2022
@DCNick3
Copy link
Author

DCNick3 commented Nov 8, 2022

I did devise my own workaround (recompiling the gdb reverting the breaking change).

@sgowroji
Copy link
Member

sgowroji commented Nov 8, 2022

Please reopen if you are looking for support

@sgowroji sgowroji closed this as completed Nov 8, 2022
@DCNick3
Copy link
Author

DCNick3 commented Nov 8, 2022

I believe that closing this issue is not the best way forward. Even though there are several workarounds (patching & building your own gdb/downgrading gdb/editing plugin's gdbserver wrapper) none of them are sustainable and all of them have their downsides.

I believe a better resolution should be pursued, like:

  • working on fix in gdb and upstreaming it (there was a patch sent to the mailing list https://sourceware.org/pipermail/gdb-patches/2021-October/182723.html, but it doesn't seem to have much activity)
  • changing the way target's stdout/stderr are handled in the plugin (I think there is no way to redirect them with the new parsing system gdb introduced)

As a proper resolution does not exist yet I believe it would be best to keep this issue open so as to signify that there is no solution yet

@sgowroji sgowroji reopened this Nov 8, 2022
@sgowroji sgowroji added awaiting-maintainer Awaiting review from Bazel team on issues and removed more-data-needed Awaiting response from issue author labels Nov 8, 2022
@sgowroji sgowroji assigned tpasternak and unassigned jastice Nov 17, 2022
emidln-imc pushed a commit to imc-trading/intellij that referenced this issue Apr 3, 2023
Gdb's argument parsing changed in a way that
prevented the redirection arguments to be handled
in its internal shell. This caused them to be passed
as the first 2 arguments for the debugged binary

The fix introduces a wrapper script that performs
the inferior output redirection internally, which is
a more stable mechanism since gdb only expects
the wrapper to call `exec "$@"` at the end

bazelbuild#2958
tpasternak pushed a commit that referenced this issue Apr 26, 2023
Gdb's argument parsing changed in a way that
prevented the redirection arguments to be handled
in its internal shell. This caused them to be passed
as the first 2 arguments for the debugged binary

The fix introduces a wrapper script that performs
the inferior output redirection internally, which is
a more stable mechanism since gdb only expects
the wrapper to call `exec "$@"` at the end

#2958

Co-authored-by: Lucas Pingas Gomes <lucas.gomes@imc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-maintainer Awaiting review from Bazel team on issues lang: c++ C++ rules integration product: CLion CLion plugin topic: debugging Issues related to debugging type: bug
Projects
None yet
Development

No branches or pull requests

7 participants