Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: "Tedeschi, Walfred" <walfred.tedeschi@intel.com>,
	       Joel Brobecker <brobecker@adacore.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [PATCH v1] Intel(R) MPX - Bound violation handling.
Date: Mon, 21 Dec 2015 17:23:00 -0000	[thread overview]
Message-ID: <56783587.6010009@redhat.com> (raw)
In-Reply-To: <AC542571535E904D8E8ADAE745D60B194450629D@IRSMSX104.ger.corp.intel.com>

Hi Walfred,

(Please don't top post.)

On 12/15/2015 10:58 AM, Tedeschi, Walfred wrote:
> From: Pedro Alves [mailto:palves@redhat.com] 
> On 12/14/2015 05:43 PM, Tedeschi, Walfred wrote:

>>> >> +  set_running (user_visible_resume_ptid (1), 0);
>> > 
>> > This is the part that _really_ concerns me, not necessary because I think it's wrong (although, it is a big red flag for me), but because I don't understand why it's needed, and how it will affect things.
>> > (From Joel)
>>> >> +  si_code = parse_and_eval_long ("$_siginfo.si_code\n");
>> > 
>> > During the debugging time I understood that inferior was stopped. Gdb is that was in the process to determine in which state the inferior was.
>> > In this sense I set the flag at this point to allow for the evaluation.
> Where is the error thrown that required brute-forcing set_running away?
> Can we try to find some other way to handle this?  E.g., use something a bit lower level than parse_and_eval_long that bypasses the error?  E.g., start from lookup_internalvar and then use type/value manipulation routines?
>
> It comes from the infrun.c (validate_siginfo_access) .
> The requirement is not running is not fulfilled. Also in the case that we execute a lookup_interval and ask for value_contents we trigger the same code.
>
> What would be the suggestion here:
> Additional function to be used internally in infrun or add a flag.

I gave this some thought, and ended up filling 2 PRs and a proposal forward:

  [Bug breakpoints/19388] Access $_siginfo in breakpoint (catch signal) condition
  https://sourceware.org/bugzilla/show_bug.cgi?id=19388

  [Bug gdb/19389] GDB sometimes mistakenly allows accessing registers of running threads
  https://sourceware.org/bugzilla/show_bug.cgi?id=19389

I think that to move forward we should change validate_siginfo_access to
check is_executing instead of is_running for now.  I think it'll
fix your case (please give it a try).  It's the same check that we do to
prevent accessing registers from a running thread.  $_siginfo is
conceptually really no different from registers -- we could think of it as
just another register.

I sent a patch that does that here, along with testcase that justifies
it on its own, independently of MPX:

  [PATCH] Fix PR19388: Can't access $_siginfo in breakpoint (catch signal) condition
  https://sourceware.org/ml/gdb-patches/2015-12/msg00443.html

By making this change, $_siginfo will become exposed to PR19389 too,
just like registers, and we should definitely fix it, but that seems
like a lesser evil than not being able to get at the info at all.

Thanks,
Pedro Alves


  parent reply	other threads:[~2015-12-21 17:23 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-26 15:09 [PATCH obv] Changing compiler flags for MPX tests Walfred Tedeschi
2015-10-26 16:10 ` [PATCH v1] Intel(R) MPX registers to the DWARF enumeration Walfred Tedeschi
2015-12-06 16:35   ` Joel Brobecker
2015-12-06 17:42     ` H.J. Lu
2015-12-07  8:29       ` Tedeschi, Walfred
2015-10-26 16:11 ` [PATCH v1] Synchronize siginfo type described in GDB with the kernel and glibc ones Walfred Tedeschi
2015-11-18 23:01   ` Joel Brobecker
2015-11-19  9:52     ` Tedeschi, Walfred
2015-11-19 13:27       ` Pedro Alves
2015-11-19 16:41         ` Tedeschi, Walfred
2015-11-19 17:07           ` Pedro Alves
2015-12-01 10:08             ` Tedeschi, Walfred
2015-12-01 12:08               ` Pedro Alves
2015-10-26 16:22 ` [PATCH v1] ABI changes for Intel(R) MPX Walfred Tedeschi
2015-10-26 19:07   ` Eli Zaretskii
2015-10-27 17:21     ` Tedeschi, Walfred
2015-12-06 16:16   ` Joel Brobecker
2015-10-26 16:25 ` [PATCH obv] Fix non stopping breakpoint on newer compilers Walfred Tedeschi
2015-11-04 14:42   ` Joel Brobecker
2015-10-26 16:26 ` [PATCH v1] Intel(R) MPX - Bound violation handling Walfred Tedeschi
2015-11-04 14:55   ` Joel Brobecker
2015-11-05 10:04     ` Tedeschi, Walfred
2015-11-19  0:01   ` Joel Brobecker
2015-12-14 17:43     ` Tedeschi, Walfred
2015-12-14 18:45       ` Pedro Alves
2015-12-15 11:01         ` Tedeschi, Walfred
2015-12-16 15:21           ` Tedeschi, Walfred
2015-12-16 16:52             ` Pedro Alves
2015-12-17 17:31               ` Tedeschi, Walfred
2015-12-21 17:23           ` Pedro Alves [this message]
2015-11-04 14:42 ` [PATCH obv] Changing compiler flags for MPX tests Joel Brobecker

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=56783587.6010009@redhat.com \
    --to=palves@redhat.com \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=walfred.tedeschi@intel.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