[patch+7.12.1,1/2] Code cleanup: write_exp_msymbol: +is_tls
Commit Message
Hi,
no functionality change, for patch 2/2.
Jan
gdb/ChangeLog
2016-10-09 Jan Kratochvil <jan.kratochvil@redhat.com>
* parse.c (write_exp_msymbol): New variable is_tls, use it.
Comments
On Sun, Oct 9, 2016 at 7:56 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> --- a/gdb/parse.c
> +++ b/gdb/parse.c
>
> /* The minimal symbol might point to a function descriptor;
> resolve it to the actual code address instead. */
> pc = gdbarch_convert_from_func_ptr_addr (gdbarch, addr, ¤t_target);
> - if (pc != addr)
> + if (!is_tls && pc != addr)
It does have functionality change. This should be moved to patch #2?
On Tue, 11 Oct 2016 16:21:58 +0200, Yao Qi wrote:
> On Sun, Oct 9, 2016 at 7:56 PM, Jan Kratochvil
> <jan.kratochvil@redhat.com> wrote:
> > --- a/gdb/parse.c
> > +++ b/gdb/parse.c
> >
> > /* The minimal symbol might point to a function descriptor;
> > resolve it to the actual code address instead. */
> > pc = gdbarch_convert_from_func_ptr_addr (gdbarch, addr, ¤t_target);
> > - if (pc != addr)
> > + if (!is_tls && pc != addr)
>
> It does have functionality change.
Ah, you are right.
> This should be moved to patch #2?
It is unrelated to patch #2, it could be a new patch #3 (or patch #1.5).
In fact if (tls) then gdbarch_convert_from_func_ptr_addr should not be even
called so this patch is also wrong (suboptimal).
Let's forget about this patch hunk now.
Jan
On 10/09/2016 07:56 PM, Jan Kratochvil wrote:
> Hi,
>
> no functionality change, for patch 2/2.
> /* The minimal symbol might point to a function descriptor;
> resolve it to the actual code address instead. */
> pc = gdbarch_convert_from_func_ptr_addr (gdbarch, addr, ¤t_target);
> - if (pc != addr)
> + if (!is_tls && pc != addr)
This bit might be a functionality change / fix, AFAICS. I pushed in
the other patch without this bit, because:
- it looked like we could skip the gdbarch_convert_from_func_ptr_addr
call too?
- I wondered whether for TLS function pointers we should
actually translate the TLS address and then
still see if the translated address points to a function
descriptor that needs to be resolved, like in the non-TLS case.
At this point, it seemed better to leave it be until we add
a testcase with a TLS function pointer.
Thanks,
Pedro Alves
@@ -484,11 +484,13 @@ write_exp_msymbol (struct parser_state *ps,
struct obj_section *section = MSYMBOL_OBJ_SECTION (objfile, msymbol);
enum minimal_symbol_type type = MSYMBOL_TYPE (msymbol);
CORE_ADDR pc;
+ const int is_tls = (section != NULL
+ && section->the_bfd_section->flags & SEC_THREAD_LOCAL);
/* The minimal symbol might point to a function descriptor;
resolve it to the actual code address instead. */
pc = gdbarch_convert_from_func_ptr_addr (gdbarch, addr, ¤t_target);
- if (pc != addr)
+ if (!is_tls && pc != addr)
{
struct bound_minimal_symbol ifunc_msym = lookup_minimal_symbol_by_pc (pc);
@@ -520,7 +522,7 @@ write_exp_msymbol (struct parser_state *ps,
write_exp_elt_longcst (ps, (LONGEST) addr);
write_exp_elt_opcode (ps, OP_LONG);
- if (section && section->the_bfd_section->flags & SEC_THREAD_LOCAL)
+ if (is_tls)
{
write_exp_elt_opcode (ps, UNOP_MEMVAL_TLS);
write_exp_elt_objfile (ps, objfile);