[PATCH] Fixes problem setting breakpoint in dynamic loader

Thiago Jung Bauermann
2007-04-27T19:22:43Z

[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]

Hi folks,

This is a re-submission of:

http://sourceware.org/ml/gdb-patches/2006-07/msg00264.html

The patch above resolves all review comments which were made on previous
versions (I collected them in the attached .txt, if you wish to have a
look). The problem is that it is malformed (maybe it was hand edited, I
don't know) and doesn't fix a couple of typos in comments.

The attached patch fixes the typos and applies cleanly on current CVS
HEAD.

Regarding this comment from Kevin Buettner (found in
http://sources.redhat.com/ml/gdb-patches/2006-06/msg00382.html):

> > + When those function descriptors are in the code section, they
> > + contain executable code and we can set a breakpoint there. >
*/
>
> Also, I don't mind that the comment was rearranged, but I would like
> to see information regarding the two linker symbols retained in some
> fashion.

The information regarding the two linker symbols is outdated. The "dot"
symbols pointing to the function entrypoint are no longer generated by
the toolchain (this is true at least for ppc, I don't know about other
architectures). Tools are expected to automatically generate those
symbols when needed.

Is this ok for inclusion?
--
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center

[-- Attachment #2: comments.txt --]
[-- Type: text/plain, Size: 3997 bytes --]

My comments in [].

--------------------------------------------------------------------------------
http://sourceware.org/ml/gdb-patches/2006-07/msg00183.html

> + if (sym_addr != 0)
> + /* Convert 'sym_addr' from a function pointer to an address. */
> + sym_addr = gdbarch_convert_from_func_ptr_addr (current_gdbarch,
> + sym_addr,
> + tmp_bfd_target);

Why do you call gdbarch_convert_from_func_ptr_addr here? This is already
called by adjust_breakpoint_address via
ppc64_sysv_abi_adjust_breakpoint_address when setting the breakpoint.

[ that code is gone now ]

Andreas.
--------------------------------------------------------------------------------
http://sourceware.org/ml/gdb-patches/2006-07/msg00020.html

Formatting notes.

On Wed, Jul 05, 2006 at 04:57:23PM -0700, PAUL GILLIAM wrote:
> + contain executable code and we can set a breakpoint there. */

Two spaces after period, here and elsewhere.

[ that code is gone now ]

> + /* No symbol was found in a code section, so look elsewhere. */
> + for (bkpt_namep = solib_break_names; *bkpt_namep!=NULL; bkpt_namep++)

Line is too long; also related, spaces around operators.

[ that code is gone now ]

> + /* On ABI's that use function descriptors that are in the data
> + section, */

Lost a bit of this comment?

[ that code is gone now ]
--
Daniel Jacobowitz
CodeSourcery
--------------------------------------------------------------------------------
http://sources.redhat.com/ml/gdb-patches/2006-06/msg00382.html

> + /* What we're looking for here is the machine code entry point,
> + so we are only interested in symbols in code sections.
> +
> + On ABI's that use function descriptors, the linker symbol with
^^^^^
ABIs
[ fixed ]

> + the same name as a C funtion points to that functions descriptor.
^^^^^^^ ^^^^^^^^^
function function's

[ that code is gone now ]

> + When those function descriptors are in the code section, they
> + contain executable code and we can set a breakpoint there. */

Also, I don't mind that the comment was rearranged, but I would like
to see information regarding the two linker symbols retained in some
fashion.

[ see message ]

> sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_CODE);
> if (sym_addr != 0)
> break;
> }
>
> + if (sym_addr == 0)
> + {
> + CORE_ADDR sect_offset;
> +
> + /* No symbol was found in a code section, so look in the data
> + sections. This will only happen when the linker symbol points
> + to a function descriptor that is in a data section. */
> + for (bkpt_namep = solib_break_names; *bkpt_namep!=NULL; bkpt_namep++)
> + {
> + /* On ABI's that use function descriptors that are in the data
> + section, */
> + sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_DATA);
> + if (sym_addr != 0)
> + break;
> + }

Starting from the line immediately below...
> + if (sym_addr == 0)
> + {
> + target_close (tmp_bfd_target, 0);
> + goto bkpt_at_symbol;
> + }
...through the line immediately above, could we delete those lines and
instead just say:

if (sym_addr != 0)

before the assignment (sym_addr = gdbarch_convert...) below?

(This gets rid of the goto and the extra call to target_close().)

[ that code is gone now ]

> +
> + /* Convert 'sym_addr' from a function pointer to an address. */
> + sym_addr = gdbarch_convert_from_func_ptr_addr (current_gdbarch,
> + sym_addr,
> + tmp_bfd_target);
> + }
> +
> /* We're done with both the temporary bfd and target. Remember,
> closing the target closes the underlying bfd. */
> target_close (tmp_bfd_target, 0);

With my suggested changes above, I think this is okay. I'd like to
see another patch posted to this list though prior to committing...

Thanks,

Kevin

[-- Attachment #3: loader-break-2007-04-27.diff --]
[-- Type: text/x-patch, Size: 10096 bytes --]

2007-04-27 Paul Gilliam <pgilliam@us.ibm.com>
Thiago Bauermann <bauerman@br.ibm.com>

* ppc-linux-tdep.c (ppc64_linux_convert_from_func_ptr_addr):
Change name and update to work for both ppc-64 and ppc-32.
(ppc_linux_init_abi): Arrange for "gdbarch_convert_from_func_ptr_addr"
to be set for both ppc-64 and ppc-32.
* solib-svr4.c (solib_break_names): Remove "._dl_debug_state" and
the comment explaining it.
(bfd_lookup_symbol): Change the meaning of the parameter "sect_flags"
so that any bit can be on instead of all bits.
(enable_break): Change the call to bfd_lookup_symbol() to search
both code and data sections and change the comment to explain why,
getting rid of any mention of 'dot' symbols.

diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index cf09a17..8c4bcb9 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -725,17 +725,25 @@ ppc64_skip_trampoline_code (CORE_ADDR pc)
}

-/* Support for CONVERT_FROM_FUNC_PTR_ADDR (ARCH, ADDR, TARG) on PPC64
+/* Support for CONVERT_FROM_FUNC_PTR_ADDR (ARCH, ADDR, TARG) on PPC
GNU/Linux.

Usually a function pointer's representation is simply the address
- of the function. On GNU/Linux on the 64-bit PowerPC however, a
- function pointer is represented by a pointer to a TOC entry. This
- TOC entry contains three words, the first word is the address of
- the function, the second word is the TOC pointer (r2), and the
- third word is the static chain value. Throughout GDB it is
- currently assumed that a function pointer contains the address of
- the function, which is not easy to fix. In addition, the
+ of the function. On GNU/Linux on the PowerPC however, a function
+ pointer is, in fact, a pointer to a function descriptor.
+
+ For PPC64, a function descriptor is a TOC entry, in a data section,
+ which contains three words: the first word is the address of the
+ function, the second word is the TOC pointer (r2), and the third word
+ is the static chain value.
+
+ For PPC32, there are two kinds of function descriptors: non-secure
+ and secure. Non-secure function descriptors are in a code section,
+ are executable and thus need no translation. Secure ones are in a
+ data section and contain one word: the address of the function.
+
+ Throughout GDB it is currently assumed that a function pointer contains
+ the address of the function, which is not easy to fix. In addition, the
conversion of a function address to a function pointer would
require allocation of a TOC entry in the inferior's memory space,
with all its drawbacks. To be able to call C++ virtual methods in
@@ -744,20 +752,45 @@ ppc64_skip_trampoline_code (CORE_ADDR pc)
from a function pointer. */

/* If ADDR points at what is clearly a function descriptor, transform
- it into the address of the corresponding function. Be
- conservative, otherwize GDB will do the transformation on any
+ it into the address of the corresponding function, if needed. Be
+ conservative, otherwise GDB will do the transformation on any
random addresses such as occures when there is no symbol table. */

static CORE_ADDR
-ppc64_linux_convert_from_func_ptr_addr (struct gdbarch *gdbarch,
- CORE_ADDR addr,
- struct target_ops *targ)
+ppc_linux_convert_from_func_ptr_addr (struct gdbarch *gdbarch,
+ CORE_ADDR addr,
+ struct target_ops *targ)
{
+ struct gdbarch_tdep *tdep;
struct section_table *s = target_section_by_addr (targ, addr);
+ char *sect_name = NULL;
+
+ if (!s)
+ return addr;
+
+ tdep = gdbarch_tdep (gdbarch);
+
+ switch (tdep->wordsize)
+ {
+ case 4:
+ sect_name = ".plt";
+ break;
+ case 8:
+ sect_name = ".opd";
+ break;
+ default:
+ internal_error (__FILE__, __LINE__,
+ _("failed internal consitency check"));
+ }

/* Check if ADDR points to a function descriptor. */
- if (s && strcmp (s->the_bfd_section->name, ".opd") == 0)
- return get_target_memory_unsigned (targ, addr, 8);
+
+ /* NOTE: this depends on the coincidence that the address of a functions
+ entry point is contained in the first word of its function descriptor
+ for both PPC-64 and for PPC-32 with secure PLTs. */
+ if ((strcmp (s->the_bfd_section->name, sect_name) == 0)
+ && s->the_bfd_section->flags & SEC_DATA)
+ return get_target_memory_unsigned (targ, addr, tdep->wordsize);

return addr;
}
@@ -1038,6 +1071,11 @@ ppc_linux_init_abi (struct gdbarch_info info,
/* NOTE: cagney/2005-01-25: True for both 32- and 64-bit. */
set_gdbarch_long_double_bit (gdbarch, 8 * TARGET_CHAR_BIT);

+ /* Handle PPC GNU/Linux function pointers (which are really function
+ descriptors). */
+ set_gdbarch_convert_from_func_ptr_addr
+ (gdbarch, ppc_linux_convert_from_func_ptr_addr);
+
if (tdep->wordsize == 4)
{
/* Until November 2001, gcc did not comply with the 32 bit SysV
@@ -1059,27 +1097,27 @@ ppc_linux_init_abi (struct gdbarch_info info,
(gdbarch, svr4_ilp32_fetch_link_map_offsets);

/* Trampolines. */
- tramp_frame_prepend_unwinder (gdbarch, &ppc32_linux_sigaction_tramp_frame);
- tramp_frame_prepend_unwinder (gdbarch, &ppc32_linux_sighandler_tramp_frame);
+ tramp_frame_prepend_unwinder (gdbarch,
+ &ppc32_linux_sigaction_tramp_frame);
+ tramp_frame_prepend_unwinder (gdbarch,
+ &ppc32_linux_sighandler_tramp_frame);
}

if (tdep->wordsize == 8)
{
- /* Handle PPC64 GNU/Linux function pointers (which are really
- function descriptors). */
- set_gdbarch_convert_from_func_ptr_addr
- (gdbarch, ppc64_linux_convert_from_func_ptr_addr);
- set_gdbarch_skip_trampoline_code (gdbarch, ppc64_skip_trampoline_code);
-
/* Shared library handling. */
+ set_gdbarch_skip_trampoline_code (gdbarch, ppc64_skip_trampoline_code);
set_solib_svr4_fetch_link_map_offsets
(gdbarch, svr4_lp64_fetch_link_map_offsets);

/* Trampolines. */
- tramp_frame_prepend_unwinder (gdbarch, &ppc64_linux_sigaction_tramp_frame);
- tramp_frame_prepend_unwinder (gdbarch, &ppc64_linux_sighandler_tramp_frame);
+ tramp_frame_prepend_unwinder (gdbarch,
+ &ppc64_linux_sigaction_tramp_frame);
+ tramp_frame_prepend_unwinder (gdbarch,
+ &ppc64_linux_sighandler_tramp_frame);
}
- set_gdbarch_regset_from_core_section (gdbarch, ppc_linux_regset_from_core_section);
+ set_gdbarch_regset_from_core_section (gdbarch,
+ ppc_linux_regset_from_core_section);

/* Enable TLS support. */
set_gdbarch_fetch_tls_load_module_address (gdbarch,
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 105ff33..678f90e 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -84,16 +84,6 @@ static char *solib_break_names[] =
"rtld_db_dlactivity",
"_rtld_debug_state",

- /* On the 64-bit PowerPC, the linker symbol with the same name as
- the C function points to a function descriptor, not to the entry
- point. The linker symbol whose name is the C function name
- prefixed with a '.' points to the function's entry point. So
- when we look through this table, we ignore symbols that point
- into the data section (thus skipping the descriptor's symbol),
- and eventually try this one, giving us the real entry point
- address. */
- "._dl_debug_state",
-
NULL
};

@@ -283,7 +273,7 @@ static CORE_ADDR bfd_lookup_symbol (bfd *, char *, flagword);
interface structures in the shared library.

If SECT_FLAGS is non-zero, only match symbols in sections whose
- flags include all those in SECT_FLAGS.
+ flags include any of those in SECT_FLAGS.

Note that 0 is specifically allowed as an error return (no
such symbol).
@@ -312,7 +302,7 @@ bfd_lookup_symbol (bfd *abfd, char *symname, flagword sect_flags)
{
sym = *symbol_table++;
if (strcmp (sym->name, symname) == 0
- && (sym->section->flags & sect_flags) == sect_flags)
+ && (sym->section->flags & sect_flags))
{
/* Bfd symbols are section relative. */
symaddr = sym->value + sym->section->vma;
@@ -341,7 +331,7 @@ bfd_lookup_symbol (bfd *abfd, char *symname, flagword sect_flags)
sym = *symbol_table++;

if (strcmp (sym->name, symname) == 0
- && (sym->section->flags & sect_flags) == sect_flags)
+ && (sym->section->flags & sect_flags))
{
/* Bfd symbols are section relative. */
symaddr = sym->value + sym->section->vma;
@@ -1094,16 +1084,19 @@ enable_break (void)
/* Now try to set a breakpoint in the dynamic linker. */
for (bkpt_namep = solib_break_names; *bkpt_namep != NULL; bkpt_namep++)
{
- /* On ABI's that use function descriptors, there are usually
- two linker symbols associated with each C function: one
- pointing at the actual entry point of the machine code,
- and one pointing at the function's descriptor. The
- latter symbol has the same name as the C function.
-
- What we're looking for here is the machine code entry
- point, so we are only interested in symbols in code
- sections. */
- sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_CODE);
+ /* On ABIs that use function descriptors, the linker symbol
+ with the same name as a C function points to a function
+ descriptor that can be in either a code or data section.
+ In either case, pointers to a function descriptor are
+ converted to "breakpointable" addresses in set_raw_breakpoint()
+ using gdbarch_adjust_breakpoint_address(), if needed.
+
+ On ABIs that don't use function descriptors, there is no
+ need for set_raw_breakpoint() to adjust the address. Also,
+ we assume that none of the symbols in the solib_break_names
+ table are in a data section. */
+ sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep,
+ SEC_CODE | SEC_DATA);
if (sym_addr != 0)
break;
}