public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@ericsson.com>
To: Joel Brobecker <brobecker@adacore.com>,
	Simon Marchi	<simon.marchi@polymtl.ca>
Cc: <gdb-patches@sourceware.org>, Keith Seitz <keiths@redhat.com>,
	Xavier Roirand <roirand@adacore.com>
Subject: Re: [RFA/linespec] wrong line number in breakpoint location
Date: Fri, 26 Jan 2018 17:16:00 -0000	[thread overview]
Message-ID: <5bc2ff63-7341-4000-8ec4-d56c87779c3d@ericsson.com> (raw)
In-Reply-To: <20171221113214.hezwvaatnbd4yzfq@adacore.com>

On 2017-12-21 06:32 AM, Joel Brobecker wrote:
> [with the patch, this time...]
> 
> On Thu, Dec 21, 2017 at 03:31:27PM +0400, Joel Brobecker wrote:
>>>> /* The following function's implementation starts by including a file
>>>>    (break-include.inc) which contains a copyright header followed by
>>>>    a single C statement.  When we break on the line where the function
>>>
>>> I would say "place a breakpoint" instead of break.  For me "to break" is the
>>> action of the program stopping on a breakpoint (though maybe it
>>
>> Sounds good.
>>
>> Here is a new version :). I also noticed I forgot the gdb.base/
>> subdir in the name of the new files in the testsuite, so I fixed
>> that up too.
>>
>> gdb/ChangeLog:
>>
>>         * linespec.c (create_sals_line_offset): Remove code that preserved
>>         the symtab_and_line's line number.
>>
>> gdb/testsuite/ChangeLog:
>>
>>         * gdb.base/break-include.c, gdb.base/break-include.inc,
>>         gdb.base/break-include.exp: New files.
>>         * gdb.base/ending-run.exp: Minor adaptations due to the breakpoint's
>>         line number now being the actual line number where the breakpoint
>>         was inserted.
>>         * gdb.mi/mi-break.exp: Likewise.
>>         * gdb.mi/mi-reverse.exp: Likewise.
>>         * gdb.mi/mi-simplerun.exp: Ditto.
>>
>> Thanks,
>> -- 
>> Joel
> 

Hi Joel,

I started seeing a failure with this patch:

FAIL: gdb.base/break.exp: verify that they were cleared

Here is the test code:

 40 int
 41 main (int argc, char **argv, char **envp)
 42 {
 43     if (argc == 12345) {  /* an unlikely value < 2^16, in case uninited */ /* set breakpoint 6 here */
 44         fprintf (stderr, "usage:  factorial <number>\n");
 45         return 1;
 46     }
 47     printf ("%d\n", factorial (atoi ("6")));  /* set breakpoint 1 here */
 48     /* set breakpoint 12 here */
 49     marker1 ();  /* set breakpoint 11 here */
 50     marker2 (43); /* set breakpoint 20 here */

What happens is that we build a binary with optimization, set a breakpoint
on line 47, and expect "info break" to show it at line 47.  In reality,
everything about line 47 has been inlined and there's no address associated to
line 47.  The following location in that file that has generated code associated
to it is line 49, so that's where the breakpoint is placed in reality.  With
this patch, "info break" therefore now shows line 49.

This particular test isn't really about testing with optimized code, it's about
checking if we can clear breakpoint commands.  So we should probably test that
against a non-optimized binary.

I am using Ubuntu 16.04's default compiler, gcc 5.4.0 (the outcome of the test probably
depends on the particular compiler used).  When I try on my Arch Linux machine and
gcc 7.2.1, the test passes (the generated address/line mapping is different, and
there's an address associated to line 47).

Simon

  parent reply	other threads:[~2018-01-26 17:16 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-18  2:44 Joel Brobecker
2017-12-18  4:09 ` Simon Marchi
2017-12-19  9:24   ` Joel Brobecker
2017-12-21  1:31     ` Simon Marchi
2017-12-21 11:31       ` Joel Brobecker
2017-12-21 11:32         ` Joel Brobecker
2018-01-22  4:17           ` pushed: " Joel Brobecker
2018-01-26 17:16           ` Simon Marchi [this message]
2018-01-29  4:45             ` Joel Brobecker
2018-01-29 17:01               ` Simon Marchi
2018-01-30  4:09                 ` Joel Brobecker
2018-01-30 12:39                   ` 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=5bc2ff63-7341-4000-8ec4-d56c87779c3d@ericsson.com \
    --to=simon.marchi@ericsson.com \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=keiths@redhat.com \
    --cc=roirand@adacore.com \
    --cc=simon.marchi@polymtl.ca \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).