public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Nelson Chu <nelson.chu@sifive.com>
To: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Binutils <binutils@sourceware.org>,
	gdb-patches@sourceware.org,  Jim Wilson <jimw@sifive.com>,
	Andrew Burgess <andrew.burgess@embecosm.com>,
	 Kito Cheng <kito.cheng@sifive.com>,
	Andrew Waterman <andrew@sifive.com>
Subject: Re: [PATCH v2 2/3] RISC-V: PR27916, Support mapping symbols.
Date: Fri, 16 Jul 2021 10:58:21 +0800	[thread overview]
Message-ID: <CAJYME4GHk4uo-yYtTEXraBEBJbCsSBpq3UBtJrdnVK+t5KouRg@mail.gmail.com> (raw)
In-Reply-To: <mhng-25bd6f14-4df8-4298-965d-d87d67f97995@palmerdabbelt-glaptop>

On Thu, Jul 15, 2021 at 11:16 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > +/* Define mapping symbols for riscv.  */
> > +
> > +bool
> > +riscv_elf_is_mapping_symbols (const char *name)
> > +{
> > +  return (!strncmp (name, "$d", 2)
> > +       || !strncmp (name, "$x", 2)
> > +       || !strncmp (name, "$a", 2));
> > +}
>
> I can't quite figure this one out: this will treat something like
> "$aWHATEVER" as a mapping symbol, does that cause binutils to crash
> somewhere?  All I'm seeing it do is mark these as hidden symbols, which
> I guess is OK (we've kind of set the "anything with $ is inernal
> precedent").
>
> Either way,
> Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
Just filtering the mapping symbols in the
bfd/elfnn-riscv.c/riscv_elf_is_target_special_symbol isn't enough.  I
notice that we also need to filter them in
bfd/elfnn-riscv.c/riscv_maybe_function_sym and
opcode/riscv-dis.c/riscv_symbol_is_valid, otherwise, something may be
broken.  For example, objudmp/nm/addr2line calls bfd_find_nearest_line
to dump something, including the GNU note sections, and it will call
_bfd_elf_find_function to get the function name.  If we don't filter
the mapping symbols here, then they may be regarded as function names,
which is not the expected result.

BTW, since the discussion from psabi,
https://github.com/riscv/riscv-elf-psabi-doc/pull/196
The $a and $d+size are abandoned for now.  So the v3 patch is a little
bit different from this one.


Thanks
Nelson

  reply	other threads:[~2021-07-16  2:58 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-09  7:28 [PATCH v2 0/3] RISC-V: The series to supporting " Nelson Chu
2021-07-09  7:28 ` [PATCH v2 1/3] RISC-V: Enable elf attributes when default configure option isn't set Nelson Chu
2021-07-09  8:00   ` Kito Cheng
2021-07-13  7:07     ` Nelson Chu
2021-07-13 20:38   ` Palmer Dabbelt
2021-07-15  1:47     ` Nelson Chu
2022-02-25 13:57   ` Sebastian Huber
2021-07-09  7:28 ` [PATCH v2 2/3] RISC-V: PR27916, Support mapping symbols Nelson Chu
2021-07-15 15:16   ` Palmer Dabbelt
2021-07-16  2:58     ` Nelson Chu [this message]
2021-07-09  7:28 ` [PATCH v2 3/3] RISC-V: PR27916, Extend .insn directive to support hardcode encoding Nelson Chu
2021-07-09  8:01   ` Kito Cheng
2021-07-14 20:38     ` Palmer Dabbelt

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=CAJYME4GHk4uo-yYtTEXraBEBJbCsSBpq3UBtJrdnVK+t5KouRg@mail.gmail.com \
    --to=nelson.chu@sifive.com \
    --cc=andrew.burgess@embecosm.com \
    --cc=andrew@sifive.com \
    --cc=binutils@sourceware.org \
    --cc=gdb-patches@sourceware.org \
    --cc=jimw@sifive.com \
    --cc=kito.cheng@sifive.com \
    --cc=palmer@dabbelt.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).