* [PATCH] [gdb] Refactor svr4_solib_ops::lm_addr_check
@ 2026-09-11 19:19 Tom de Vries
0 siblings, 0 replies; only message in thread
From: Tom de Vries @ 2026-09-11 19:19 UTC (permalink / raw)
To: gdb-patches
I came across svr4_solib_ops::lm_addr_check and found it hard to follow.
In particular, it's indent-happy and minimizes code using a goto, resulting in
the following end-of-function:
...
}
}
set_addr:
li.l_addr = l_addr;
li.l_addr_p = 1;
}
return li.l_addr;
}
...
I could just address the indentation, but I'd be adding a goto:
...
- if (dynaddr + l_addr != l_dynaddr)
+ if (dynaddr + l_addr == l_dynaddr)
+ goto set_addr;
...
Instead, refactor the function into this form:
...
svr4_solib_ops::lm_addr_check (const solib &so, bfd *abfd) const
{
auto &li = get_lm_info_svr4 (so);
if (li.l_addr_p)
return li.l_addr;
auto set_addr = [&li] (CORE_ADDR addr)
{
li.l_addr = addr;
li.l_addr_p = true;
return addr;
}
if (...)
return set_addr (l_addr);
...
return set_addr (l_addr);
}
...
---
gdb/solib-svr4.c | 160 +++++++++++++++++++++++------------------------
1 file changed, 80 insertions(+), 80 deletions(-)
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index c4af1b11a4d..40b0fc55113 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -225,106 +225,106 @@ CORE_ADDR
svr4_solib_ops::lm_addr_check (const solib &so, bfd *abfd) const
{
auto &li = get_lm_info_svr4 (so);
+ if (li.l_addr_p)
+ return li.l_addr;
- if (!li.l_addr_p)
- {
- struct bfd_section *dyninfo_sect;
- CORE_ADDR l_addr, l_dynaddr, dynaddr;
+ auto set_addr = [&li] (CORE_ADDR addr)
+ {
+ li.l_addr = addr;
+ li.l_addr_p = true;
+ return addr;
+ };
- l_addr = li.l_addr_inferior;
+ struct bfd_section *dyninfo_sect;
+ CORE_ADDR l_addr, l_dynaddr, dynaddr;
- if (!abfd || !this->has_lm_dynamic_from_link_map ())
- goto set_addr;
+ l_addr = li.l_addr_inferior;
+ if (!abfd || !this->has_lm_dynamic_from_link_map ())
+ return set_addr (l_addr);
- l_dynaddr = li.l_ld;
+ l_dynaddr = li.l_ld;
- dyninfo_sect = bfd_get_section_by_name (abfd, ".dynamic");
- if (dyninfo_sect == NULL)
- goto set_addr;
+ dyninfo_sect = bfd_get_section_by_name (abfd, ".dynamic");
+ if (dyninfo_sect == NULL)
+ return set_addr (l_addr);
- dynaddr = bfd_section_vma (dyninfo_sect);
+ dynaddr = bfd_section_vma (dyninfo_sect);
+ if (dynaddr + l_addr == l_dynaddr)
+ return set_addr (l_addr);
- if (dynaddr + l_addr != l_dynaddr)
- {
- CORE_ADDR align = 0x1000;
- CORE_ADDR minpagesize = align;
+ CORE_ADDR align = 0x1000;
+ CORE_ADDR minpagesize = align;
- if (bfd_get_flavour (abfd) == bfd_target_elf_flavour)
- {
- Elf_Internal_Ehdr *ehdr = elf_tdata (abfd)->elf_header;
- Elf_Internal_Phdr *phdr = elf_tdata (abfd)->phdr;
- int i;
+ if (bfd_get_flavour (abfd) == bfd_target_elf_flavour)
+ {
+ Elf_Internal_Ehdr *ehdr = elf_tdata (abfd)->elf_header;
+ Elf_Internal_Phdr *phdr = elf_tdata (abfd)->phdr;
+ int i;
- align = 1;
+ align = 1;
- for (i = 0; i < ehdr->e_phnum; i++)
- if (phdr[i].p_type == PT_LOAD && phdr[i].p_align > align)
- align = phdr[i].p_align;
+ for (i = 0; i < ehdr->e_phnum; i++)
+ if (phdr[i].p_type == PT_LOAD && phdr[i].p_align > align)
+ align = phdr[i].p_align;
- minpagesize = get_elf_backend_data (abfd)->minpagesize;
- }
+ minpagesize = get_elf_backend_data (abfd)->minpagesize;
+ }
- /* Turn it into a mask. */
- align--;
+ /* Turn it into a mask. */
+ align--;
- /* If the changes match the alignment requirements, we
- assume we're using a core file that was generated by the
- same binary, just prelinked with a different base offset.
- If it doesn't match, we may have a different binary, the
- same binary with the dynamic table loaded at an unrelated
- location, or anything, really. To avoid regressions,
- don't adjust the base offset in the latter case, although
- odds are that, if things really changed, debugging won't
- quite work.
+ /* If the changes match the alignment requirements, we
+ assume we're using a core file that was generated by the
+ same binary, just prelinked with a different base offset.
+ If it doesn't match, we may have a different binary, the
+ same binary with the dynamic table loaded at an unrelated
+ location, or anything, really. To avoid regressions,
+ don't adjust the base offset in the latter case, although
+ odds are that, if things really changed, debugging won't
+ quite work.
- One could expect more the condition
- ((l_addr & align) == 0 && ((l_dynaddr - dynaddr) & align) == 0)
- but the one below is relaxed for PPC. The PPC kernel supports
- either 4k or 64k page sizes. To be prepared for 64k pages,
- PPC ELF files are built using an alignment requirement of 64k.
- However, when running on a kernel supporting 4k pages, the memory
- mapping of the library may not actually happen on a 64k boundary!
+ One could expect more the condition
+ ((l_addr & align) == 0 && ((l_dynaddr - dynaddr) & align) == 0)
+ but the one below is relaxed for PPC. The PPC kernel supports
+ either 4k or 64k page sizes. To be prepared for 64k pages,
+ PPC ELF files are built using an alignment requirement of 64k.
+ However, when running on a kernel supporting 4k pages, the memory
+ mapping of the library may not actually happen on a 64k boundary!
- (In the usual case where (l_addr & align) == 0, this check is
- equivalent to the possibly expected check above.)
+ (In the usual case where (l_addr & align) == 0, this check is
+ equivalent to the possibly expected check above.)
- Even on PPC it must be zero-aligned at least for MINPAGESIZE. */
+ Even on PPC it must be zero-aligned at least for MINPAGESIZE. */
- l_addr = l_dynaddr - dynaddr;
+ l_addr = l_dynaddr - dynaddr;
- if ((l_addr & (minpagesize - 1)) == 0
- && (l_addr & align) == ((l_dynaddr - dynaddr) & align))
- {
- if (info_verbose)
- gdb_printf (_("Using PIC (Position Independent Code) "
- "prelink displacement %s for \"%s\".\n"),
- paddress (current_inferior ()->arch (), l_addr),
- so.name.c_str ());
- }
- else
- {
- /* There is no way to verify the library file matches. prelink
- can during prelinking of an unprelinked file (or unprelinking
- of a prelinked file) shift the DYNAMIC segment by arbitrary
- offset without any page size alignment. There is no way to
- find out the ELF header and/or Program Headers for a limited
- verification if it they match. One could do a verification
- of the DYNAMIC segment. Still the found address is the best
- one GDB could find. */
-
- warning (_(".dynamic section for \"%s\" "
- "is not at the expected address "
- "(wrong library or version mismatch?)"),
- so.name.c_str ());
- }
- }
-
- set_addr:
- li.l_addr = l_addr;
- li.l_addr_p = 1;
+ if ((l_addr & (minpagesize - 1)) == 0
+ && (l_addr & align) == ((l_dynaddr - dynaddr) & align))
+ {
+ if (info_verbose)
+ gdb_printf (_("Using PIC (Position Independent Code) "
+ "prelink displacement %s for \"%s\".\n"),
+ paddress (current_inferior ()->arch (), l_addr),
+ so.name.c_str ());
+ }
+ else
+ {
+ /* There is no way to verify the library file matches. prelink
+ can during prelinking of an unprelinked file (or unprelinking
+ of a prelinked file) shift the DYNAMIC segment by arbitrary
+ offset without any page size alignment. There is no way to
+ find out the ELF header and/or Program Headers for a limited
+ verification if it they match. One could do a verification
+ of the DYNAMIC segment. Still the found address is the best
+ one GDB could find. */
+
+ warning (_(".dynamic section for \"%s\" "
+ "is not at the expected address "
+ "(wrong library or version mismatch?)"),
+ so.name.c_str ());
}
- return li.l_addr;
+ return set_addr (l_addr);
}
struct svr4_so
base-commit: 0855b93cfb911a4feab76a31bb1914982b3e6979
--
2.51.0
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2026-09-11 19:20 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-11 19:19 [PATCH] [gdb] Refactor svr4_solib_ops::lm_addr_check Tom de Vries
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox