Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Sergio Durigan Junior <sergiodj@redhat.com>
To: Pedro Alves <palves@redhat.com>
Cc: gdb-patches@sourceware.org, 	Joel Brobecker <brobecker@adacore.com>
Subject: Re: [PATCH 2/2] Don't lose language determined from the "main" name (fix gdb.ada/minsyms.exp)
Date: Tue, 21 Nov 2017 16:23:00 -0000	[thread overview]
Message-ID: <87a7zfob1w.fsf@redhat.com> (raw)
In-Reply-To: <1511280661-14725-3-git-send-email-palves@redhat.com> (Pedro	Alves's message of "Tue, 21 Nov 2017 16:11:01 +0000")

On Tuesday, November 21 2017, Pedro Alves wrote:

> gdb.ada/minsyms.exp fails like this here:
>
>  FAIL: gdb.ada/minsyms.exp: print integer(some_minsym)
>  FAIL: gdb.ada/minsyms.exp: print /x integer(&some_minsym)
>
> The problem is that if you have debug info for glibc, GDB switches the
> current language to C before it reaches the program's entry point, and
> then Ada's cast syntax doesn't work when the current language is C:
>
>   print integer(some_minsym)
>   A syntax error in expression, near `some_minsym)'.
>   (gdb) FAIL: gdb.ada/minsyms.exp: print integer(some_minsym)
>
> I first thought of doing "set language ada" in the testcase, but
> looking deeper, I realized that before running to main, GDB knows the
> program is Ada, determined by reading __gnat_ada_main_program_name,
> via set_initial_language->main_language->find_main_name->
> ada_main_name, and loses that when it is handling a shared library
> event.  That looks like a bug to me.
>
> gdb/testsuite/ChangeLog:
> 2017-11-21  Pedro Alves  <palves@redhat.com>
>
> 	* solib-svr4.c: Save/restore language around evaluating a probe
> 	argument.
> ---
>  gdb/solib-svr4.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
> index 5ec606d..e6f818a 100644
> --- a/gdb/solib-svr4.c
> +++ b/gdb/solib-svr4.c
> @@ -1936,6 +1936,21 @@ svr4_handle_solib_event (void)
>    usm_chain = make_cleanup (resume_section_map_updates_cleanup,
>  			    current_program_space);
>  
> +  /* Make sure evaluating probe arguments doesn't cause us to switch
> +     the user's current language to the runtime's language.
> +     Evaluating probe arguments relies on reading registers off the
> +     selected frame.  When we're handling a shared library event, this
> +     is going to be the first time we fetch the selected frame (as
> +     opposed to the current frame), and if there's debug info for the
> +     loader (e.g., glibc), this switches to its language (usually C).
> +     Usually that ends up masked because we will usually next stop in
> +     the main program (e.g., user did "start"), and switch to the
> +     right language again then, if the program has debug info.
> +     However, if the program does not have debug info, then GDB won't
> +     switch, and we'd lose the language that was determined earlier by
> +     sniffing the program's main name.  */
> +  scoped_restore_current_language save_language;
> +
>    TRY
>      {
>        val = evaluate_probe_argument (pa->probe, 1, frame);

Hey, thanks for the patch.

Since this is guaranteed to be an stap probe, WDYT about moving this
scoped_restore_current_language to
stap-probe.c:stap_evaluate_probe_argument?  This way we won't be bit by
this problem in other parts that also evaluate arguments of probes.

Arguably, this should be set for every probe type IMHO, but it's fine if
we just do it for stap probes for now.

Cheers,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/


  reply	other threads:[~2017-11-21 16:23 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-21 16:11 [PATCH 0/2] Fix a couple gdb.ada/minsyms.exp problems Pedro Alves
2017-11-21 16:11 ` [PATCH 2/2] Don't lose language determined from the "main" name (fix gdb.ada/minsyms.exp) Pedro Alves
2017-11-21 16:23   ` Sergio Durigan Junior [this message]
2017-11-21 16:42     ` Pedro Alves
2017-11-21 16:53       ` Sergio Durigan Junior
2017-11-21 16:56       ` Pedro Alves
2017-11-21 17:05         ` Pedro Alves
2017-11-21 17:15         ` Sergio Durigan Junior
2017-11-21 17:22           ` Pedro Alves
2017-11-21 16:11 ` [PATCH 1/2] gdb.ada/minsyms.exp: Don't hardcode the variable's address Pedro Alves
2017-11-21 16:24   ` Sergio Durigan Junior
2017-11-21 16:50   ` Joel Brobecker
2017-11-21 17:08     ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a7zfob1w.fsf@redhat.com \
    --to=sergiodj@redhat.com \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox