Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Joel Brobecker <brobecker@adacore.com>
Cc: gdb-patches@sourceware.org
Subject: Re: Fix gdb.ada/bp_c_mixed_case.exp (PR gdb/22670) (Re: [PATCH 3/3] Add new gdb.ada/bp_c_mixed_case testcase for PR gdb/22670)
Date: Mon, 08 Jan 2018 15:00:00 -0000	[thread overview]
Message-ID: <f2581935-af98-ea32-6c1e-82a85364485a@redhat.com> (raw)
In-Reply-To: <20180108035724.gac5u77znunzhho3@adacore.com>

On 01/08/2018 03:57 AM, Joel Brobecker wrote:
> On Fri, Jan 05, 2018 at 04:34:39PM +0000, Pedro Alves wrote:

>> Below's a fix for this one.
> 
> Thanks!
> 
> I confirm the test now passes for me as well :). 

Great!

> I have a question though:
> 
>> >From 439f8c51ff8f6cd9fb3bbc330a40492a15992add Mon Sep 17 00:00:00 2001
>> From: Pedro Alves <palves@redhat.com>
>> Date: Fri, 5 Jan 2018 00:17:19 +0000
>> Subject: [PATCH 1/2] Fix gdb.ada/bp_c_mixed_case.exp (PR gdb/22670)
>>
>> The problem here is that we were using the user-provided lookup name
>> literally for linkage name comparisons.  I.e., "<MixedCase>" with the
>> "<>"s included.  That obviously can't work since the "<>" are not
>> really part of the linkage name.  The original idea was that we'd use
>> the symbol's language to select the right symbol name matching
>> algorithm, but that doesn't work for Ada because it's not really
>> possible to unambiguously tell from the linkage name alone whether
>> we're dealing with Ada symbols, so Ada minsyms end up with no language
>> set, or sometimes C++ set.  So fix this by treating Ada mode specially
>> when determining the linkage name to match against.
> 
> I am wondering why minimal symbols are involved in this case,
> considering that the C file was build with debugging information.
> Shouldn't we be getting the function's address from the partial/full
> symtabs instead?

AFAIK, GDB always worked this way for linespecs, even before my C++
wildmatching patches -- we collect symbols from both debug info and
minsyms, and coalesce them by address to avoid duplicates
(linespec.c:add_matching_symbols_to_info).  

The completion paths then try to do the same thing (though implemented
differently) [1].

I think it makes sense because:

- even if you have debug information in the binary, the debug information
  won't cover all function symbols.  Some may be internal linker-/compiler-
  generated symbols.  Or..

- ..there may be multiple symbols with the same name in different
  compilation units that all end up in the same binary/objfile.  Some may
  have debug info while others not.  E.g. (C, but applies to any language,
  or mixed languages):

static void function () {}                // in file1.c, compiled with -g
static int function (int p) { return p; } // in file2.c, compiled WITHOUT -g

  I could easily see the 'with'/'without -g' functions ending up both in
  the same objfile via static linking, for example.  We want "b <function>" to
  set a breakpoint on all the functions.

[1] - the C++ wildmatching series eliminated the divergence a bit, but
there's still a lot more duplication that there should ideally be.
One of the points of the gdb.linespec/cpcompletion.exp and
gdb.linespec/cpls-*.exp testcases is making sure that completion
and actually setting a breakpoint finds the same locations.

Let me know if this answers your question.

Thanks,
Pedro Alves


  reply	other threads:[~2018-01-08 15:00 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-04  8:36 FYI/pushed: Additional tests showing regression post C++ wild matching Joel Brobecker
2018-01-04  8:36 ` [PATCH 2/3] Add "complete break ada" test to gdb.ada/complete.exp Joel Brobecker
2018-01-05 16:37   ` [PATCH] Fix gdb.ada/complete.exp's "complete break ada" test (PR, gdb/22670) (Re: [PATCH 2/3] Add "complete break ada" test to gdb.ada/complete.exp) Pedro Alves
2018-01-08  4:05     ` Joel Brobecker
2018-01-04  8:36 ` [PATCH 3/3] Add new gdb.ada/bp_c_mixed_case testcase for PR gdb/22670 Joel Brobecker
2018-01-05 16:34   ` Fix gdb.ada/bp_c_mixed_case.exp (PR gdb/22670) (Re: [PATCH 3/3] Add new gdb.ada/bp_c_mixed_case testcase for PR gdb/22670) Pedro Alves
2018-01-08  3:57     ` Joel Brobecker
2018-01-08 15:00       ` Pedro Alves [this message]
2018-01-09  9:46         ` Joel Brobecker
2018-01-09 14:59           ` Pedro Alves
2018-01-09 16:45             ` Pedro Alves
2018-01-09 17:22               ` Pedro Alves
2018-01-10  3:36               ` Joel Brobecker
2018-01-10 23:41                 ` Pedro Alves
2018-01-11  4:00                   ` Joel Brobecker
2018-01-04  8:36 ` [PATCH 1/3] Add gdb.ada/info_addr_mixed_case new testcase Joel Brobecker
2018-01-04 13:25   ` Pedro Alves
2018-01-04 18:33     ` Pedro Alves
2018-01-05  3:22       ` Joel Brobecker
2018-01-05 16:06         ` 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=f2581935-af98-ea32-6c1e-82a85364485a@redhat.com \
    --to=palves@redhat.com \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /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