This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] MIPS: fix mips16 symbol identification
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: Shun-Yen Lu <dark dot asparagus at gmail dot com>
- Cc: <gdb-patches at sourceware dot org>
- Date: Thu, 19 Apr 2012 13:15:47 +0100
- Subject: Re: [PATCH] MIPS: fix mips16 symbol identification
- References: <CAKC5APGA=7wS-19EPLq2MAmyrmw=2a-6prSndqMm+qkLiUu_6g@mail.gmail.com>
Hi Shun-Yen,
> A mips16 symbol may contains non-zero visibility bits, so original
> implementation
> cannot correctly identify mips16 internal/hidden/protected symbols.
>
> The fix below uses ELF_ST_IS_MIPS16 macro to check mips16 symbol instead of
> checking st_other directly.
Thank you for your contribution. Interestingly enough I have a similar
change included with a larger set that I'll be pushing in the near future.
Until then your change is of course perfectly valid, however it has a
couple of small problems, as noted below.
> 2012-04-18 Shun-Yen Lu <dark.asparagus@gmail.com>
>
> gdb/
> * mips-tdep.c (mips_elf_make_msymbol_special): Fix identification
> of mips16 symbol.
s/symbol/symbols/ -- I think that would sound (read) better.
> --- a/gdb/mips-tdep.c
> +++ b/gdb/mips-tdep.c
> @@ -268,7 +268,7 @@ mips_abi_regsize (struct gdbarch *gdbarch)
> ?static void
> ?mips_elf_make_msymbol_special (asymbol * sym, struct minimal_symbol *msym)
> ?{
> -? if (((elf_symbol_type *) (sym))->internal_elf_sym.st_other == STO_MIPS16)
> +? if (ELF_ST_IS_MIPS16(((elf_symbol_type *) (sym))->internal_elf_sym.st_other))
> ???? {
> ?????? MSYMBOL_TARGET_FLAG_1 (msym) = 1;
> ???? }
You need to follow the GNU coding standard, there has to be a space
between ELF_ST_IS_MIPS16 and the following opening bracked (just as with
MSYMBOL_TARGET_FLAG_1 below), and then you'll have to wrap the line as it
won't fit in 79 columns anymore (keeping the indentation of the
continuation line correct). Please resubmit with these changes made.
I've noticed you are not listed in the MAINTAINERS file, do you have a
copyright assignment in place with FSF and repository write access? If
not, then I am going to accept this change as trivial enough not to
require the assignment, however if you plan to make further contributions,
then you'll need to make such an assignment. I can give you detailed
instructions if interested.
Maciej