From: Fangrui Song <maskray@google.com>
To: Nicholas Guriev <nicholas@guriev.su>
Cc: libc-alpha@sourceware.org
Subject: Re: [PATCH] elf: Rewrite long RESOLVE_MAP macro to a debug friendly function
Date: Mon, 2 May 2022 14:20:35 -0700 [thread overview]
Message-ID: <20220502212035.45lvk22rjui65pvq@google.com> (raw)
In-Reply-To: <daf4d62304825d43b82311c530b44282879feb7a.1651512228.git.nicholas@guriev.su>
On 2022-05-02, Nicholas Guriev wrote:
>A static function that may be inlined is way better to find where exactly a
>crash happens. So one can step into the function with GDB. The macro still
>remains for compatibility with other code.
I think this is useful. Does the size of dl-reloc.os code increase or
decrease with the change?
>Signed-off-by: Nicholas Guriev <nicholas@guriev.su>
>---
>
>I was playing with the DT_AUXILIARY flag and run into a crash in the
>RESOLVE_MAP macro. Alas, GDB is unable to see what is going on in a
>multi-line macro, so I turned it to a static function. Hope this is useful.
>
> elf/dl-reloc.c | 55 ++++++++++++++++++++++++++++++--------------------
> 1 file changed, 33 insertions(+), 22 deletions(-)
>
>diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
>index 771a34bd14..88d7fee041 100644
>--- a/elf/dl-reloc.c
>+++ b/elf/dl-reloc.c
>@@ -162,29 +162,40 @@ _dl_nothread_init_static_tls (struct link_map *map)
> }
> #endif /* !PTHREAD_IN_LIBC */
>
>+static lookup_t
>+_dl_resolve_map (lookup_t l, struct r_scope_elem *scope[], const ElfW(Sym) **ref,
>+ const struct r_found_version *version, unsigned long r_type)
>+{
>+ if (ELFW(ST_BIND) ((*ref)->st_info) == STB_LOCAL
>+ || __glibc_unlikely (dl_symbol_visibility_binds_local_p (*ref)))
>+ return l;
>+
>+ if (__glibc_unlikely (*ref == l->l_lookup_cache.sym)
>+ && elf_machine_type_class (r_type) == l->l_lookup_cache.type_class)
>+ {
>+ bump_num_cache_relocations ();
>+ *ref = l->l_lookup_cache.ret;
>+ }
>+ else
>+ {
>+ lookup_t _lr;
>+ int _tc = elf_machine_type_class (r_type);
>+ l->l_lookup_cache.type_class = _tc;
>+ l->l_lookup_cache.sym = *ref;
>+ const struct r_found_version *v = NULL;
>+ if (version != NULL && version->hash != 0)
>+ v = version;
>+ _lr = _dl_lookup_symbol_x ((const char *) D_PTR (l, l_info[DT_STRTAB]) + (*ref)->st_name,
>+ l, ref, scope, v, _tc,
>+ DL_LOOKUP_ADD_DEPENDENCY | DL_LOOKUP_FOR_RELOCATE, NULL);
Use `lookup_t _lr = ` ?
>+ l->l_lookup_cache.ret = *ref;
>+ l->l_lookup_cache.value = _lr;
>+ }
>+ return l->l_lookup_cache.value;
>+}
>+
> /* This macro is used as a callback from the ELF_DYNAMIC_RELOCATE code. */
>-#define RESOLVE_MAP(l, scope, ref, version, r_type) \
>- ((ELFW(ST_BIND) ((*ref)->st_info) != STB_LOCAL \
>- && __glibc_likely (!dl_symbol_visibility_binds_local_p (*ref))) \
>- ? ((__glibc_unlikely ((*ref) == l->l_lookup_cache.sym) \
>- && elf_machine_type_class (r_type) == l->l_lookup_cache.type_class) \
>- ? (bump_num_cache_relocations (), \
>- (*ref) = l->l_lookup_cache.ret, \
>- l->l_lookup_cache.value) \
>- : ({ lookup_t _lr; \
>- int _tc = elf_machine_type_class (r_type); \
>- l->l_lookup_cache.type_class = _tc; \
>- l->l_lookup_cache.sym = (*ref); \
>- const struct r_found_version *v = NULL; \
>- if ((version) != NULL && (version)->hash != 0) \
>- v = (version); \
>- _lr = _dl_lookup_symbol_x ((const char *) D_PTR (l, l_info[DT_STRTAB]) + (*ref)->st_name, \
>- l, (ref), scope, v, _tc, \
>- DL_LOOKUP_ADD_DEPENDENCY \
>- | DL_LOOKUP_FOR_RELOCATE, NULL); \
>- l->l_lookup_cache.ret = (*ref); \
>- l->l_lookup_cache.value = _lr; })) \
>- : l)
>+#define RESOLVE_MAP _dl_resolve_map
>
> #include "dynamic-link.h"
>
>--
>2.32.0
>
next prev parent reply other threads:[~2022-05-02 21:20 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-02 14:51 Nicholas Guriev
2022-05-02 21:20 ` Fangrui Song [this message]
2022-05-03 10:04 ` Nicholas Guriev
2022-05-03 20:12 ` Fāng-ruì Sòng
2022-05-07 13:33 ` [PATCH v3] " Nicholas Guriev
2022-05-09 13:38 ` Siddhesh Poyarekar
2022-05-14 14:27 ` Nicholas Guriev
2022-05-14 22:48 ` Fangrui Song
2022-05-15 0:58 ` Siddhesh Poyarekar
2022-05-22 22:24 ` Fangrui Song
2022-05-23 7:28 ` Siddhesh Poyarekar
2022-05-23 12:35 ` Adhemerval Zanella
2022-05-23 13:02 ` Siddhesh Poyarekar
2022-05-23 16:22 ` Adhemerval Zanella
2022-05-23 16:33 ` Siddhesh Poyarekar
2022-05-23 16:56 ` Adhemerval Zanella
2022-05-03 19:14 ` [PATCH v2] " Nicholas Guriev
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=20220502212035.45lvk22rjui65pvq@google.com \
--to=maskray@google.com \
--cc=libc-alpha@sourceware.org \
--cc=nicholas@guriev.su \
/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).