[PATCH] PR28873 - Implement eu-readelf -D
>From bdc19de94bff8f8812611b9ba8c0116a650d0fb5 Mon Sep 17 00:00:00 2001
From: Di Chen
Date: Fri, 13 Jan 2023 20:12:43 +0800
Subject: [PATCH] readelf: display dynamic symtab without section headers
This commit adds a new option "-D/--use-dynamic" to support printing the
dynamic symbol table from the PT_DYNAMIC segment. By using the
PT_DYNAMIC segment, eu-readelf can go through the contents of dynamic
section entries and the values of each tag. From that, we can get the
address and size of the dynamic symbol table, the address of the string
table, etc.
By using the new option "-D/--use-dynamic", eu-readelf can list the
symbols without section headers.
Example:
$ ./src/readelf -Ds a.out
0: 0 NOTYPE LOCAL DEFAULTUNDEF
1: 0 FUNCGLOBAL DEFAULTUNDEF
__libc_start_main@GLIBC_2.34 (2)
2: 0 NOTYPE WEAK DEFAULTUNDEF
__gmon_start__
https://sourceware.org/bugzilla/show_bug.cgi?id=28873
Signed-off-by: Di Chen
---
src/readelf.c | 289 --
1 file changed, 283 insertions(+), 6 deletions(-)
diff --git a/src/readelf.c b/src/readelf.c
index 451f8400..9e1e9b73 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -307,7 +307,8 @@ static void handle_relocs_rel (Ebl *ebl, GElf_Ehdr
*ehdr, Elf_Scn *scn,
static void handle_relocs_rela (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn,
GElf_Shdr *shdr);
static bool print_symtab (Ebl *ebl, int type);
-static void handle_symtab (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr);
+static bool handle_symtab (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr);
+static bool handle_dynamic_symtab (Ebl *ebl);
static void print_verinfo (Ebl *ebl);
static void handle_verneed (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr);
static void handle_verdef (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr);
@@ -327,7 +328,9 @@ enum dyn_idx
{
i_strsz,
i_verneed,
+ i_verneednum,
i_verdef,
+ i_verdefnum,
i_versym,
i_symtab,
i_strtab,
@@ -1042,7 +1045,7 @@ process_elf_file (Dwfl_Module *dwflmod, int fd)
symtab_printed |= print_symtab (ebl, SHT_DYNSYM);
if (print_version_info)
print_verinfo (ebl);
- if (print_symbol_table)
+ if (print_symbol_table && !use_dynamic_segment)
symtab_printed |= print_symtab (ebl, SHT_SYMTAB);
if ((print_symbol_table || print_dynsym_table)
@@ -2442,6 +2445,12 @@ handle_relocs_rela (Ebl *ebl, GElf_Ehdr *ehdr,
Elf_Scn *scn, GElf_Shdr *shdr)
static bool
print_symtab (Ebl *ebl, int type)
{
+ /* Use the dynamic section info to display symbol tables. */
+ if (use_dynamic_segment && type == SHT_DYNSYM)
+ {
+ return handle_dynamic_symtab(ebl);
+ }
+
/* Find the symbol table(s). For this we have to search through the
section table. */
Elf_Scn *scn = NULL;
@@ -2480,16 +2489,275 @@ print_symtab (Ebl *ebl, int type)
_("cannot get section [%zd] header: %s"),
elf_ndxscn (scn), elf_errmsg (-1));
}
- handle_symtab (ebl, scn, shdr);
- symtab_printed = true;
+ symtab_printed = handle_symtab (ebl, scn, shdr);
}
}
return symtab_printed;
}
+static bool
+handle_dynamic_symtab (Ebl *ebl)
+{
+ GElf_Phdr *phdr = NULL;
+ /* phnum is a static variable which already fetched in function
process_elf_file. */
+ for (size_t i = 0; i < phnum; ++i) {
+GElf_Phdr phdr_mem;
+phdr = gelf_getphdr(ebl->elf, i, &phdr_mem);
+if (phdr->p_type == PT_DYNAMIC) {
+ break;
+}
+ }
+ if (phdr == NULL)
+ return false;
-static void
+ GElf_Addr addrs[i_max] = {0,};
+ GElf_Off offs[i_max] = {0,};
+ get_dynscn_addrs(ebl->elf, phdr, addrs);
+ find_offsets(ebl->elf, 0, i_max, addrs, offs);
+
+ size_t syments;
+
+ GElf_Ehdr ehdr_mem;
+ GElf_Ehdr *ehdr = gelf_getehdr(ebl->elf, &ehdr_mem);
+
+ if (offs[i_hash] != 0) {
+/* In the original format, .hash says the size of .dynsym. */
+
+size_t entsz = SH_ENTSIZE_HASH(ehdr);
+Elf_Data *data =
+elf_getdata_rawchunk(ebl->elf, offs[i_hash] + entsz, entsz,
+ (entsz == 4 ? ELF_T_WORD : ELF_T_XWORD));
+if (data != NULL)
+ syments = (entsz == 4 ? *(const GElf_Word *)data->d_buf
+: *(const GElf_Xword *)data->d_buf);
+ }
+ if (offs[i_gnu_hash] != 0 && syments == 0) {
+/* In the new format, we can derive it with some work. */
+
+const struct {
+ Elf32_Word nbuckets;
+ Elf32_Word symndx;
+ Elf32_Word maskwords;
+ Elf32_Word shift2;
+} * header;
+
+Elf_Data *data = elf_getdata_rawchunk(ebl->elf, offs[i_gnu_hash],
+ sizeof *header, ELF_T_WORD);
+if (data != NULL) {
+ header = data->d_buf;
+ Elf32_Word nbuckets = header->nbuckets;
+ Elf32_Word symndx = header->symndx;
+ GElf_Off buckets_at = (offs[i_gnu_hash] + sizeof *header +
+ (gelf_getclass(ebl->elf) * sizeof(Elf32_Word)
*
+ header->maskwords));
+
+ //
Re: [PATCH] PR28873 - Implement eu-readelf -D
GElf_Off hasharr_at
+ = (buckets_at + nbuckets * sizeof (Elf32_Word));
+ hasharr_at += (maxndx - symndx) * sizeof (Elf32_Word);
+ do
+{
+ data = elf_getdata_rawchunk (
+ ebl->elf, hasharr_at, sizeof (Elf32_Word),
ELF_T_WORD);
+ if (data != NULL && (*(const Elf32_Word *)data->d_buf &
1u))
+{
+ syments = maxndx + 1;
+ break;
+}
+ ++maxndx;
+ hasharr_at += sizeof (Elf32_Word);
+}
+ while (data != NULL);
+}
+}
+}
+ if (offs[i_strtab] > offs[i_symtab] && syments == 0)
+syments = ((offs[i_strtab] - offs[i_symtab])
+ / gelf_fsize (ebl->elf, ELF_T_SYM, 1, EV_CURRENT));
- vd_offset += verdef->vd_next;
- verdef = (verdef->vd_next == 0
- ? NULL
- : gelf_getverdef (verdef_data, vd_offset,
- &verdef_mem));
-}
+ if (syments <= 0 || offs[i_strtab] == 0 || offs[i_symtab] == 0)
+{
+ error_exit (0, _ ("Dynamic symbol information is not available for "
+"displaying symbols."));
+}
- if (verdef != NULL)
-{
- GElf_Verdaux verdaux_mem;
- GElf_Verdaux *verdaux
- = gelf_getverdaux (verdef_data,
- vd_offset + verdef->vd_aux,
- &verdaux_mem);
-
- if (verdaux != NULL)
- printf ((*versym & 0x8000) ? "@%s" : "@@%s",
- elf_strptr (ebl->elf, verdef_stridx,
-verdaux->vda_name));
-}
- }
-}
- }
+ /* All the data chunk initializaion. */
+ Elf_Data *symdata = NULL;
+ Elf_Data *symstrdata = NULL;
+ Elf_Data *versym_data = NULL;
+ Elf_Data *verdef_data = NULL;
+ Elf_Data *verneed_data = NULL;
- putchar_unlocked ('\n');
-}
+ symdata = elf_getdata_rawchunk (
+ ebl->elf, offs[i_symtab],
+ gelf_fsize (ebl->elf, ELF_T_SYM, syments, EV_CURRENT), ELF_T_SYM);
+ symstrdata = elf_getdata_rawchunk (ebl->elf, offs[i_strtab],
addrs[i_strsz],
+ ELF_T_BYTE);
+ versym_data = elf_getdata_rawchunk (
+ ebl->elf, offs[i_versym], syments * sizeof (Elf64_Half), ELF_T_HALF);
+
+ /* Get the verneed_data without vernaux. */
+ verneed_data = elf_getdata_rawchunk (
+ ebl->elf, offs[i_verneed], addrs[i_verneednum] * sizeof
(Elf64_Verneed),
+ ELF_T_VNEED);
+ size_t vernauxnum = 0;
+ size_t vn_next_offset = 0;
+
+ for (size_t i = 0; i < addrs[i_verneednum]; i++)
+{
+ GElf_Verneed *verneed
+ = (GElf_Verneed *)(verneed_data->d_buf + vn_next_offset);
+ vernauxnum += verneed->vn_cnt;
+ vn_next_offset += verneed->vn_next;
+}
+
+ /* Update the verneed_data to include the vernaux. */
+ verneed_data = elf_getdata_rawchunk (
+ ebl->elf, offs[i_verneed],
+ (addrs[i_verneednum] + vernauxnum) * sizeof (GElf_Verneed),
ELF_T_VNEED);
+
+ /* Get the verdef_data without verdaux. */
+ verdef_data = elf_getdata_rawchunk (
+ ebl->elf, offs[i_verdef], addrs[i_verdefnum] * sizeof (Elf64_Verdef),
+ ELF_T_VDEF);
+ size_t verdauxnum = 0;
+ size_t vd_next_offset = 0;
+
+ for (size_t i = 0; i < addrs[i_verdefnum]; i++)
+{
+ GElf_Verdef *verdef
+ = (GElf_Verdef *)(verdef_data->d_buf + vd_next_offset);
+ verdauxnum += verdef->vd_cnt;
+ vd_next_offset += verdef->vd_next;
+}
+
+ /* Update the verdef_data to include the verdaux. */
+ verdef_data = elf_getdata_rawchunk (
+ ebl->elf, offs[i_verdef],
+ (addrs[i_verdefnum] + verdauxnum) * sizeof (GElf_Verdef),
ELF_T_VDEF);
+
+ unsigned int nsyms = (unsigned int)syments;
+ process_symtab (ebl, nsyms, 0, 0, 0, symdata, versym_data, symstrdata,
+ verneed_data, verdef_data, NULL);
+ return true;
}
@@ -4990,13 +5208,24 @@ get_dynscn_addrs(Elf *elf, GElf_Phdr *phdr,
GElf_Addr addrs[i_max])
addrs[i_verdef] = dyn->d_un.d_ptr;
break;
+case DT_VERDEFNUM:
+ addrs[i_verdefnum] = dyn->d_un.d_val;
+ break;
+
case DT_VERNEED:
addrs[i_verneed] = dyn->d_un.d_ptr;
break;
+case DT_VERNEEDNUM:
+ addrs[i_verneednum] = dyn->d_un.d_val;
+ break;
+
case DT_STRSZ:
addrs[i_strsz] = dyn->d_un.d_val;
break;
+case DT_SYMTAB_SHNDX:
+ addrs[i_symtab_shndx] = dyn->d_un.d_ptr;
+ break;
}
}
}
--
2.39.2
On Fri, Feb 17, 2023 at 12:12 AM Mark Wielaard wrote:
> Hi,
>
> On Sat, 2023-02-11 at 00:17 +0800, Di Chen via Elfutils-devel wrote:
> > From bdc19de94bff8f8812611b9ba8c0116a650d0fb5 Mon Sep 17 00:00:00 2001
> > From: Di Chen
> > Date: Fri, 13 Jan 2023 20:12:43 +0800
> > Subject: [PATCH] readelf: display dynamic symtab without section headers
> >
> > This commit adds a new
[PATCH] debuginfod: PR27917 - protect against federation loops
>From a726d9868f4e02d390b9071180b0c3728da3750e Mon Sep 17 00:00:00 2001
From: Di Chen
Date: Sun, 8 Aug 2021 16:57:12 +0800
Subject: [PATCH] debuginfod: PR27917 - protect against federation loops
If someone misconfigures a debuginfod federation to have loops, and a
nonexistent
buildid lookup is attempted, bad things will happen, as is documented.
This patch aims to reduce the risk by adding an option to debuginfod that
functions
kind of like an IP packet's TTL: a limit on the length of XFF: header that
debuginfod
is willing to process. If X-Forwarded-For: exceeds N hops, it will not
delegate a local
lookup miss to upstream debuginfods.
Commit ab38d167c40c99 causes federation loops for non-existent resources to
result
in multiple temporary livelocks, each lasting for $DEBUGINFOD_TIMEOUT
seconds.
Since concurrent requests for each unique resource are now serialized,
federation
loops can result in one server thread waiting to acquire a lock while the
server
thread holding the lock waits for the first thread to respond to an http
request.
This PR can help protect against the above multiple temporary livelocks
behaviour.
Ex. if --forwarded-ttl-limit=0 then the timeout behaviour of local loops
should
be avoided.
https://sourceware.org/bugzilla/show_bug.cgi?id=27917
Signed-off-by: Di Chen
---
debuginfod/debuginfod.cxx| 30 --
doc/debuginfod.8 | 6 ++
tests/run-debuginfod-find.sh | 42 +++-
3 files changed, 75 insertions(+), 3 deletions(-)
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 4ddd9255..bd103d9f 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -375,6 +375,8 @@ static const struct argp_option options[] =
#define ARGP_KEY_FDCACHE_PREFETCH_FDS 0x1006
{ "fdcache-prefetch-fds", ARGP_KEY_FDCACHE_PREFETCH_FDS, "NUM",
0,"Number of files allocated to the \
prefetch cache.", 0},
+#define ARGP_KEY_FORWARDED_TTL_LIMIT 0x1007
+ {"forwarded-ttl-limit", ARGP_KEY_FORWARDED_TTL_LIMIT, "NUM", 0, "Limit
of X-Forwarded-For hops, default 8.", 0},
{ NULL, 0, NULL, 0, NULL, 0 },
};
@@ -422,6 +424,7 @@ static long fdcache_prefetch;
static long fdcache_mintmp;
static long fdcache_prefetch_mbs;
static long fdcache_prefetch_fds;
+static unsigned forwarded_ttl_limit = 8;
static string tmpdir;
static void set_metric(const string& key, double value);
@@ -554,6 +557,9 @@ parse_opt (int key, char *arg,
if( fdcache_mintmp > 100 || fdcache_mintmp < 0 )
argp_failure(state, 1, EINVAL, "fdcache mintmp percent");
break;
+case ARGP_KEY_FORWARDED_TTL_LIMIT:
+ forwarded_ttl_limit = (unsigned) atoi(arg);
+ break;
case ARGP_KEY_ARG:
source_paths.insert(string(arg));
break;
@@ -1769,7 +1775,8 @@ handle_buildid (MHD_Connection* conn,
const string& buildid /* unsafe */,
const string& artifacttype /* unsafe */,
const string& suffix /* unsafe */,
-int *result_fd)
+int *result_fd,
+bool disable_query_server = false)
{
// validate artifacttype
string atype_code;
@@ -1862,6 +1869,12 @@ handle_buildid (MHD_Connection* conn,
// We couldn't find it in the database. Last ditch effort
// is to defer to other debuginfo servers.
+ // if X-Forwarded-For: exceeds N hops,
+ // do not delegate a local lookup miss to upstream debuginfods.
+ if (disable_query_server)
+throw reportable_exception(MHD_HTTP_NOT_FOUND, "not found,
--forwared-ttl-limit reached \
+and will not query the upstream servers");
+
int fd = -1;
debuginfod_client *client = debuginfod_pool_begin ();
if (client != NULL)
@@ -2119,6 +2132,7 @@ handler_cb (void * /*cls*/,
struct timespec ts_start, ts_end;
clock_gettime (CLOCK_MONOTONIC, &ts_start);
double afteryou = 0.0;
+ bool disable_query_server = false;
try
{
@@ -2131,6 +2145,17 @@ handler_cb (void * /*cls*/,
if (slash1 != string::npos && url1 == "/buildid")
{
+ // check if X-Forwarded-For exceeds the limit number of hops.
+ string xff = MHD_lookup_connection_value (connection,
MHD_HEADER_KIND, "X-Forwarded-For") ?: "";
+
+ unsigned int xff_count = 0;
+ for (auto&& i : xff){
+if (i == ',') xff_count++;
+ }
+
+ if (xff_count >= forwarded_ttl_limit)
+disable_query_server = true;
+
// PR27863: block this thread awhile if another thread is
already busy
// fetching the exact same thing. This is better for Everyone.
// The latecomer says "... after you!" and waits.
@@ -2171,7 +2196,7 @@ handler_cb (void * /*cls*/,
// get the resulting fd so we can report its size
int fd;
- r = handle_buildid(connection, buildid, artifacttype, suffix,
&fd);
+ r = handle_buildid(connection, buildid, artifacttype, suffix,
&fd, disable_query_serve
Re: [PATCH] debuginfod: PR27917 - protect against federation loops
Hey Frank,
1) moved the XFF check to handle_buildid.
2) replace "livelock" with "deadlock" in the commit message.
- dichen
On Thu, Aug 19, 2021 at 6:55 AM Frank Ch. Eigler wrote:
> Hi -
>
> > This patch aims to reduce the risk by adding an option to debuginfod
> > that functions kind of like an IP packet's TTL: a limit on the
> > length of XFF: header that debuginfod is willing to process. If
> > X-Forwarded-For: exceeds N hops, it will not delegate a local lookup
> > miss to upstream debuginfods. [...]
>
> Thank you very much!
>
>
> > Commit ab38d167c40c99 causes federation loops for non-existent
> > resources to result in multiple temporary livelocks, each lasting
> > for $DEBUGINFOD_TIMEOUT seconds. [...]
>
> (FWIW, the term "livelock" is not quite right here, try just
> "deadlock".)
>
> The patch looks functional, and thank you also for including the
> docs and test case. Thorough enough!
>
>
> > @@ -1862,6 +1869,12 @@ handle_buildid (MHD_Connection* conn,
> >// We couldn't find it in the database. Last ditch effort
> >// is to defer to other debuginfo servers.
> >
> > + // if X-Forwarded-For: exceeds N hops,
> > + // do not delegate a local lookup miss to upstream debuginfods.
> > + if (disable_query_server)
> > +throw reportable_exception(MHD_HTTP_NOT_FOUND, "not found,
> > --forwared-ttl-limit reached \
> > +and will not query the upstream servers");
>
> One part I don't understand is why you added the code to check for XFF
> length into handler_cb(), and then passed the disable_query_server
> result flag to this function. Was there some reason not to perform
> the XFF comma-counting right here?
>
>
> - FChE
>
>
From a0b3d4ba3d15b83d23d3594b614c8e72b87e626c Mon Sep 17 00:00:00 2001
From: Di Chen
Date: Fri, 20 Aug 2021 13:03:21 +0800
Subject: [PATCH] debuginfod: PR27917 - protect against federation loops
If someone misconfigures a debuginfod federation to have loops, and
a nonexistent buildid lookup is attempted, bad things will happen,
as is documented.
This patch aims to reduce the risk by adding an option to debuginfod
that functions kind of like an IP packet's TTL: a limit on the length of
XFF: header that debuginfod is willing to process. If X-Forwarded-For:
exceeds N hops, it will not delegate a local lookup miss to upstream
debuginfods.
Commit ab38d167c40c99 causes federation loops for non-existent resources
to result in multiple temporary deadlocks, each lasting for $DEBUGINFOD_TIMEOUT
seconds. Since concurrent requests for each unique resource are now
serialized, federation loops can result in one server thread waiting to
acquire a lock while the server thread holding the lock waits for the
first thread to respond to an http request.
This PR can help protect against the above multiple temporary deadlocks
behaviour. Ex. if --forwarded-ttl-limit=0 then the timeout behaviour of
local loops should be avoided.
https://sourceware.org/bugzilla/show_bug.cgi?id=27917
Signed-off-by: Di Chen
---
debuginfod/debuginfod.cxx| 18
doc/debuginfod.8 | 6 ++
tests/run-debuginfod-find.sh | 42 +++-
3 files changed, 65 insertions(+), 1 deletion(-)
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 4ddd9255..261049f2 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -375,6 +375,8 @@ static const struct argp_option options[] =
#define ARGP_KEY_FDCACHE_PREFETCH_FDS 0x1006
{ "fdcache-prefetch-fds", ARGP_KEY_FDCACHE_PREFETCH_FDS, "NUM", 0,"Number of files allocated to the \
prefetch cache.", 0},
+#define ARGP_KEY_FORWARDED_TTL_LIMIT 0x1007
+ {"forwarded-ttl-limit", ARGP_KEY_FORWARDED_TTL_LIMIT, "NUM", 0, "Limit of X-Forwarded-For hops, default 8.", 0},
{ NULL, 0, NULL, 0, NULL, 0 },
};
@@ -422,6 +424,7 @@ static long fdcache_prefetch;
static long fdcache_mintmp;
static long fdcache_prefetch_mbs;
static long fdcache_prefetch_fds;
+static unsigned forwarded_ttl_limit = 8;
static string tmpdir;
static void set_metric(const string& key, double value);
@@ -554,6 +557,9 @@ parse_opt (int key, char *arg,
if( fdcache_mintmp > 100 || fdcache_mintmp < 0 )
argp_failure(state, 1, EINVAL, "fdcache mintmp percent");
break;
+case ARGP_KEY_FORWARDED_TTL_LIMIT:
+ forwarded_ttl_limit = (unsigned) atoi(arg);
+ break;
case ARGP_KEY_ARG:
source_paths.insert(string(arg));
break;
@@ -1881,6 +1887,17 @@ handle_buildid (MHD_Connection* conn,
if (xff != "")
xff += string(", "); // comma separated list
+ unsigned int xff_count = 0;
+ for (auto&& i : xff){
+if (i == ',') xff_count++;
+ }
+
+ // if X-Forwarded-For: exceeds N hops,
+ // do not delegate a local lookup miss to upstream debuginfods.
+ if (xff_count >= forwarded_ttl_limit)
+throw reportable_exception(MHD_HTTP_NOT_FOUND, "not found, --forwared-ttl-lim
[PATCH] PR27940 - The /* pc=0x... */ is no longer printed by "stap -v -L 'kernel.function("*")'
>From a98718a1b97357e53cef966ed9826c69e159f46e Mon Sep 17 00:00:00 2001
From: Di Chen
Date: Thu, 2 Sep 2021 12:52:47 +0800
Subject: [PATCH] The /* pc=0x... */ is no longer printed by "stap -v -L
'kernel.function("*")'
The disappeared /* pc=0x... */ resulted from the missing implementation
of the function "dwarf_derived_probe::printsig_nonest".
Which makes "p->printsig_nonest(sig)" in main.cxx end up calling
"derived_probe::printsig_nonest", and the type of "p" is
(gdb) ptype /m p
type = /* real type = dwarf_derived_probe * */
This patch added "dwarf_derived_probe::printsig_nonest" for PC value
print.
https://sourceware.org/bugzilla/show_bug.cgi?id=27940
Signed-off-by: Di Chen
---
elaborate.h | 2 +-
tapsets.cxx | 11 +++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/elaborate.h b/elaborate.h
index 9817172a7..baa29e67c 100644
--- a/elaborate.h
+++ b/elaborate.h
@@ -204,7 +204,7 @@ struct derived_probe: public probe
virtual probe_point* sole_location () const;
virtual probe_point* script_location () const;
virtual void printsig (std::ostream &o) const cxx_override;
- void printsig_nonest (std::ostream &o) const;
+ virtual void printsig_nonest (std::ostream &o) const;
// return arguments of probe if there
virtual void getargs (std::list &) const {}
void printsig_nested (std::ostream &o) const;
diff --git a/tapsets.cxx b/tapsets.cxx
index 68b75610e..27d8995ce 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -560,6 +560,7 @@ struct dwarf_derived_probe: public
generic_kprobe_derived_probe
bool access_vars;
void printsig (std::ostream &o) const;
+ void printsig_nonest (std::ostream &o) const;
virtual void join_group (systemtap_session& s);
void emit_probe_local_init(systemtap_session& s, translator_output * o);
void getargs(std::list &arg_set) const;
@@ -5386,6 +5387,16 @@ dwarf_derived_probe::printsig (ostream& o) const
}
+void
+dwarf_derived_probe::printsig_nonest (ostream& o) const
+{
+ sole_location()->print (o);
+ if (symbol_name != "")
+o << " /* pc=<" << symbol_name << "+" << offset << "> */";
+ else
+o << " /* pc=" << section << "+0x" << hex << addr << dec << " */";
+}
+
void
dwarf_derived_probe::join_group (systemtap_session& s)
--
2.31.1
[PATCH] debuginfod: PR28242 - extend server http-response metrics
>From a574dfaf5d5636cbb7159a0118eb30e2c4aaa301 Mon Sep 17 00:00:00 2001
From: Di Chen
Date: Fri, 1 Oct 2021 22:03:41 +0800
Subject: [PATCH] debuginfod: PR28242 - extend server http-response metrics
This patch aims to extend http_responses_* metrics with another label
"type" by getting the extra artifact-type content added as a new key=value
tag.
https://sourceware.org/bugzilla/show_bug.cgi?id=28242
Signed-off-by: Di Chen
---
debuginfod/debuginfod.cxx | 55 --
tests/run-debuginfod-000-permission.sh | 12 +++---
2 files changed, 49 insertions(+), 18 deletions(-)
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 2b9a1c41..6a469b21 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -2080,6 +2080,33 @@ add_metric(const string& metric,
// and more for higher arity labels if needed
+static void
+inc_metric(const string& metric,
+ const string& lname, const string& lvalue,
+ const string& rname, const string& rvalue)
+{
+ string key = (metric + "{" + metric_label(lname, lvalue) + ",");
+ if (rvalue != "debuginfo" && rvalue != "executable" && rvalue !=
"source")
+key = (key + metric_label(rname, "") + "}");
+ else
+key = (key + metric_label(rname, rvalue) + "}");
+ unique_lock lock(metrics_lock);
+ metrics[key] ++;
+}
+static void
+add_metric(const string& metric,
+ const string& lname, const string& lvalue,
+ const string& rname, const string& rvalue,
+ double value)
+{
+ string key = (metric + "{" + metric_label(lname, lvalue) + ",");
+ if (rvalue != "debuginfo" && rvalue != "executable" && rvalue !=
"source")
+key = (key + metric_label(rname, "") + "}");
+ else
+key = (key + metric_label(rname, rvalue) + "}");
+ unique_lock lock(metrics_lock);
+ metrics[key] += value;
+}
static struct MHD_Response*
handle_metrics (off_t* size)
@@ -2165,6 +2192,7 @@ handler_cb (void * /*cls*/,
struct timespec ts_start, ts_end;
clock_gettime (CLOCK_MONOTONIC, &ts_start);
double afteryou = 0.0;
+ string artifacttype, suffix;
try
{
@@ -2203,7 +2231,7 @@ handler_cb (void * /*cls*/,
string buildid = url_copy.substr(slash1+1, slash2-slash1-1);
size_t slash3 = url_copy.find('/', slash2+1);
- string artifacttype, suffix;
+
if (slash3 == string::npos)
{
artifacttype = url_copy.substr(slash2+1);
@@ -2275,17 +2303,20 @@ handler_cb (void * /*cls*/,
// related prometheus metrics
string http_code_str = to_string(http_code);
if (http_size >= 0)
-add_metric("http_responses_transfer_bytes_sum","code",http_code_str,
- http_size);
- inc_metric("http_responses_transfer_bytes_count","code",http_code_str);
-
-
add_metric("http_responses_duration_milliseconds_sum","code",http_code_str,
- deltas*1000); // prometheus prefers _seconds and floating
point
-
inc_metric("http_responses_duration_milliseconds_count","code",http_code_str);
-
-
add_metric("http_responses_after_you_milliseconds_sum","code",http_code_str,
- afteryou*1000);
-
inc_metric("http_responses_after_you_milliseconds_count","code",http_code_str);
+add_metric("http_responses_transfer_bytes_sum",
+ "code", http_code_str, "type", artifacttype, http_size);
+ inc_metric("http_responses_transfer_bytes_count",
+ "code", http_code_str, "type", artifacttype);
+
+ add_metric("http_responses_duration_milliseconds_sum",
+ "code", http_code_str, "type", artifacttype, deltas*1000); //
prometheus prefers _seconds and floating point
+ inc_metric("http_responses_duration_milliseconds_count",
+ "code", http_code_str, "type", artifacttype);
+
+ add_metric("http_responses_after_you_milliseconds_sum",
+ "code", http_code_str, "type", artifacttype, afteryou*1000);
+ inc_metric("http_responses_after_you_milliseconds_count",
+ "code", http_code_str, "type", artifacttype);
return rc;
}
diff --git a/tests/run-debuginfod-000-permission.sh
b/tests/run-debuginfod-000-permission.sh
index 1e92bdb8..afa7e931 100755
--- a/tests/run-debuginfod-000-permission.sh
+++ b/tests/run-debuginfod-000-permission.sh
@@ -61,22 +61,22 @@ if [ -r $DEBUGINFOD_CACHE_PATH/01234567/debuginfo ];
then
err
fi
-bytecount_before=`curl -s http://127.0.0.1:$PORT1/metrics | grep
'http_responses_transfer_bytes_count{code="404"}'`
+bytecount_before=`curl -s http://127.0.0.1:$PORT1/metrics | grep
'http_responses_transfer_bytes_count{code="404",type="debuginfo"}'`
testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo 01234567
|| true
-bytecount_after=`curl -s http://127.0.0.1:$PORT1/metrics | grep
'http_responses_transfer_bytes_count{code="404"}'`
+bytecount_after=`curl -s http://127.0.0.1:$PORT1/metrics | grep
'http_responses_transfer_bytes_count{code="404",type="debuginfo"}'`
if [ "$bytecount_before" != "$bytecount_after" ]; then
- ech
[PATCH] readelf: PR28928 - wrong dynamic section entry number
commit 978663c5323cf402cd35b8614e41f24b587cbdd8 (HEAD -> dichen/DT_NULL,
origin/dichen/DT_NULL)
Author: Di Chen
Date: Tue Mar 1 20:44:38 2022 +0800
readelf: PR28928 - wrong dynamic section entry number
when using `$ eu-readelf -d {file}` to get the number of dynamic
section entris, It wrongly counts the padding DT_NULLs as dynamic
section entries. However, DT_NULL Marks end of dynamic section.
They should not be counted as dynamic section entries.
https://sourceware.org/bugzilla/show_bug.cgi?id=28928
Signed-off-by: Di Chen
diff --git a/src/readelf.c b/src/readelf.c
index 93fb5989..1bec3aa6 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -296,6 +296,7 @@ static void print_shdr (Ebl *ebl, GElf_Ehdr *ehdr);
static void print_phdr (Ebl *ebl, GElf_Ehdr *ehdr);
static void print_scngrp (Ebl *ebl);
static void print_dynamic (Ebl *ebl);
+static void handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr);
static void print_relocs (Ebl *ebl, GElf_Ehdr *ehdr);
static void handle_relocs_rel (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn,
GElf_Shdr *shdr);
@@ -1781,16 +1782,54 @@ print_dt_posflag_1 (int class, GElf_Xword d_val)
[dichen@arpeggio elfutils]$ git format-patch -1 HEAD
0001-readelf-PR28928-wrong-dynamic-section-entry-number.patch
[dichen@arpeggio elfutils]$ vim
0001-readelf-PR28928-wrong-dynamic-section-entry-number.patch
[dichen@arpeggio elfutils]$ cat
0001-readelf-PR28928-wrong-dynamic-section-entry-number.patch
>From 978663c5323cf402cd35b8614e41f24b587cbdd8 Mon Sep 17 00:00:00 2001
From: Di Chen
Date: Tue, 1 Mar 2022 20:44:38 +0800
Subject: [PATCH] readelf: PR28928 - wrong dynamic section entry number
when using `$ eu-readelf -d {file}` to get the number of dynamic
section entris, It wrongly counts the padding DT_NULLs as dynamic
section entries. However, DT_NULL Marks end of dynamic section.
They should not be counted as dynamic section entries.
https://sourceware.org/bugzilla/show_bug.cgi?id=28928
Signed-off-by: Di Chen
---
src/readelf.c | 49 --
tests/alldts.c | 5 +++--
tests/run-alldts.sh| 2 +-
tests/run-readelf-d.sh | 7 +-
4 files changed, 48 insertions(+), 15 deletions(-)
diff --git a/src/readelf.c b/src/readelf.c
index 93fb5989..1bec3aa6 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -296,6 +296,7 @@ static void print_shdr (Ebl *ebl, GElf_Ehdr *ehdr);
static void print_phdr (Ebl *ebl, GElf_Ehdr *ehdr);
static void print_scngrp (Ebl *ebl);
static void print_dynamic (Ebl *ebl);
+static void handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr);
static void print_relocs (Ebl *ebl, GElf_Ehdr *ehdr);
static void handle_relocs_rel (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn,
GElf_Shdr *shdr);
@@ -1781,16 +1782,54 @@ print_dt_posflag_1 (int class, GElf_Xword d_val)
}
+static GElf_Phdr *
+get_dyn_phdr (Elf *elf)
+{
+ GElf_Phdr *phdr = NULL;
+ for (size_t i = 0; i < phnum; ++i) {
+GElf_Phdr phdr_mem;
+phdr = gelf_getphdr(elf, i, &phdr_mem);
+if (phdr->p_type == PT_DYNAMIC) {
+ break;
+}
+ }
+ return phdr;
+}
+
+
+static size_t
+get_dyn_scnents (Elf *elf, GElf_Phdr * dyn_phdr)
+{
+ Elf_Data *data = elf_getdata_rawchunk(
+ elf, dyn_phdr->p_offset, dyn_phdr->p_filesz, ELF_T_DYN);
+ GElf_Dyn *dyn;
+ size_t dyn_idx = 0;
+ do
+ {
+GElf_Dyn dyn_mem;
+dyn = gelf_getdyn(data, dyn_idx, &dyn_mem);
+++dyn_idx;
+ } while (dyn->d_tag != DT_NULL);
+
+ return dyn_idx;
+}
+
+
static void
handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr)
{
int class = gelf_getclass (ebl->elf);
+ GElf_Phdr *dyn_phdr;
GElf_Shdr glink_mem;
GElf_Shdr *glink;
Elf_Data *data;
size_t cnt;
size_t shstrndx;
- size_t sh_entsize;
+ size_t dyn_scnents;
+
+ /* Calculate the dynamic section entry number */
+ dyn_phdr = get_dyn_phdr (ebl->elf);
+ dyn_scnents = get_dyn_scnents (ebl->elf, dyn_phdr);
/* Get the data of the section. */
data = elf_getdata (scn, NULL);
@@ -1802,8 +1841,6 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr
*shdr)
error (EXIT_FAILURE, 0,
_("cannot get section header string table index"));
- sh_entsize = gelf_fsize (ebl->elf, ELF_T_DYN, 1, EV_CURRENT);
-
glink = gelf_getshdr (elf_getscn (ebl->elf, shdr->sh_link), &glink_mem);
if (glink == NULL)
error (EXIT_FAILURE, 0, _("invalid sh_link value in section %zu"),
@@ -1813,15 +1850,15 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr
*shdr)
\nDynamic segment contains %lu entry:\n Addr: %#0*" PRIx64 " Offset:
%#08" PRIx64 " Link to section: [%2u] '%s'\n",
"\
\nDynamic segment contains %lu entries:\n Addr: %#0*" PRIx64 " Offset:
%#08" PRIx64 " Link to section: [%2u] '%s'\n",
-shdr->sh_size / sh_entsize),
- (unsigned long int) (shdr->sh_size / sh_entsize),
+dyn_scnents),
+ (unsigned long int) dyn_scnents,
class == ELFCLASS32 ? 10 : 18, shdr->sh_addr,
shdr->sh_offset,
(int) shdr-
Re: [PATCH] readelf: PR28928 - wrong dynamic section entry number
Hey team,
I made some changes for this patch:
(1) update the commit message to make it more clear
(2) tests/alldts.c needs the padding spaces for output comparison
On Tue, Mar 1, 2022 at 8:54 PM Di Chen wrote:
> commit 978663c5323cf402cd35b8614e41f24b587cbdd8 (HEAD -> dichen/DT_NULL,
> origin/dichen/DT_NULL)
> Author: Di Chen
> Date: Tue Mar 1 20:44:38 2022 +0800
>
> readelf: PR28928 - wrong dynamic section entry number
>
> when using `$ eu-readelf -d {file}` to get the number of dynamic
> section entris, It wrongly counts the padding DT_NULLs as dynamic
> section entries. However, DT_NULL Marks end of dynamic section.
> They should not be counted as dynamic section entries.
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=28928
>
> Signed-off-by: Di Chen
>
> diff --git a/src/readelf.c b/src/readelf.c
> index 93fb5989..1bec3aa6 100644
> --- a/src/readelf.c
> +++ b/src/readelf.c
> @@ -296,6 +296,7 @@ static void print_shdr (Ebl *ebl, GElf_Ehdr *ehdr);
> static void print_phdr (Ebl *ebl, GElf_Ehdr *ehdr);
> static void print_scngrp (Ebl *ebl);
> static void print_dynamic (Ebl *ebl);
> +static void handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr);
> static void print_relocs (Ebl *ebl, GElf_Ehdr *ehdr);
> static void handle_relocs_rel (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn,
>GElf_Shdr *shdr);
> @@ -1781,16 +1782,54 @@ print_dt_posflag_1 (int class, GElf_Xword d_val)
> [dichen@arpeggio elfutils]$ git format-patch -1 HEAD
> 0001-readelf-PR28928-wrong-dynamic-section-entry-number.patch
> [dichen@arpeggio elfutils]$ vim
> 0001-readelf-PR28928-wrong-dynamic-section-entry-number.patch
> [dichen@arpeggio elfutils]$ cat
> 0001-readelf-PR28928-wrong-dynamic-section-entry-number.patch
> From 978663c5323cf402cd35b8614e41f24b587cbdd8 Mon Sep 17 00:00:00 2001
> From: Di Chen
> Date: Tue, 1 Mar 2022 20:44:38 +0800
> Subject: [PATCH] readelf: PR28928 - wrong dynamic section entry number
>
> when using `$ eu-readelf -d {file}` to get the number of dynamic
> section entris, It wrongly counts the padding DT_NULLs as dynamic
> section entries. However, DT_NULL Marks end of dynamic section.
> They should not be counted as dynamic section entries.
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=28928
>
> Signed-off-by: Di Chen
> ---
> src/readelf.c | 49 --
> tests/alldts.c | 5 +++--
> tests/run-alldts.sh| 2 +-
> tests/run-readelf-d.sh | 7 +-
> 4 files changed, 48 insertions(+), 15 deletions(-)
>
> diff --git a/src/readelf.c b/src/readelf.c
> index 93fb5989..1bec3aa6 100644
> --- a/src/readelf.c
> +++ b/src/readelf.c
> @@ -296,6 +296,7 @@ static void print_shdr (Ebl *ebl, GElf_Ehdr *ehdr);
> static void print_phdr (Ebl *ebl, GElf_Ehdr *ehdr);
> static void print_scngrp (Ebl *ebl);
> static void print_dynamic (Ebl *ebl);
> +static void handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr);
> static void print_relocs (Ebl *ebl, GElf_Ehdr *ehdr);
> static void handle_relocs_rel (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn,
> GElf_Shdr *shdr);
> @@ -1781,16 +1782,54 @@ print_dt_posflag_1 (int class, GElf_Xword d_val)
> }
>
>
> +static GElf_Phdr *
> +get_dyn_phdr (Elf *elf)
> +{
> + GElf_Phdr *phdr = NULL;
> + for (size_t i = 0; i < phnum; ++i) {
> +GElf_Phdr phdr_mem;
> +phdr = gelf_getphdr(elf, i, &phdr_mem);
> +if (phdr->p_type == PT_DYNAMIC) {
> + break;
> +}
> + }
> + return phdr;
> +}
> +
> +
> +static size_t
> +get_dyn_scnents (Elf *elf, GElf_Phdr * dyn_phdr)
> +{
> + Elf_Data *data = elf_getdata_rawchunk(
> + elf, dyn_phdr->p_offset, dyn_phdr->p_filesz, ELF_T_DYN);
> + GElf_Dyn *dyn;
> + size_t dyn_idx = 0;
> + do
> + {
> +GElf_Dyn dyn_mem;
> +dyn = gelf_getdyn(data, dyn_idx, &dyn_mem);
> +++dyn_idx;
> + } while (dyn->d_tag != DT_NULL);
> +
> + return dyn_idx;
> +}
> +
> +
> static void
> handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr)
> {
>int class = gelf_getclass (ebl->elf);
> + GElf_Phdr *dyn_phdr;
>GElf_Shdr glink_mem;
>GElf_Shdr *glink;
>Elf_Data *data;
>size_t cnt;
>size_t shstrndx;
> - size_t sh_entsize;
> + size_t dyn_scnents;
> +
> + /* Calculate the dynamic section entry number */
> + dyn_phdr = get_dyn_phdr (ebl->elf);
> + dyn_scnents = get_dyn_scnents (ebl->elf, dyn_phdr);
>
>/* Get the data of the section. */
>data = elf_getdata (scn, NULL);
> @@ -1802,8 +1841,6 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr
> *shdr)
> error (EXIT_FAILURE, 0,
> _("cannot get section header string table index"));
>
> - sh_entsize = gelf_fsize (ebl->elf, ELF_T_DYN, 1, EV_CURRENT);
> -
>glink = gelf_getshdr (elf_getscn (ebl->elf, shdr->sh_link), &glink_mem);
>if (glink == NULL)
> error (EXIT_FAILURE, 0, _("invalid sh_link value in section %zu"),
> @@ -1813,15 +1850,15 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr
> *shdr)
> \
Re: [PATCH] readelf: PR28928 - wrong dynamic section entry number
>From b0da0a6f6c9a57a37a144a806ecd219a76c66b54 Mon Sep 17 00:00:00 2001
From: Di Chen
Date: Tue, 1 Mar 2022 20:44:38 +0800
Subject: [PATCH] readelf: Don't consider padding DT_NULL as dynamic section
entry
when using `$ eu-readelf -d {FILE}` to get the number of dynamic
section entris, it wrongly counts the padding DT_NULLs as dynamic
section entries. However, DT_NULL Marks end of dynamic section.
They should not be considered as dynamic section entries.
https://sourceware.org/bugzilla/show_bug.cgi?id=28928
Signed-off-by: Di Chen
---
src/readelf.c | 49 --
tests/alldts.c | 5 +++--
tests/run-alldts.sh| 2 +-
tests/run-readelf-d.sh | 7 +-
4 files changed, 48 insertions(+), 15 deletions(-)
diff --git a/src/readelf.c b/src/readelf.c
index 93fb5989..0d70bb47 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -296,6 +296,7 @@ static void print_shdr (Ebl *ebl, GElf_Ehdr *ehdr);
static void print_phdr (Ebl *ebl, GElf_Ehdr *ehdr);
static void print_scngrp (Ebl *ebl);
static void print_dynamic (Ebl *ebl);
+static void handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr);
static void print_relocs (Ebl *ebl, GElf_Ehdr *ehdr);
static void handle_relocs_rel (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn,
GElf_Shdr *shdr);
@@ -1781,16 +1782,54 @@ print_dt_posflag_1 (int class, GElf_Xword d_val)
}
+static GElf_Phdr *
+get_dyn_phdr (Elf *elf)
+{
+ GElf_Phdr *phdr = NULL;
+ for (size_t i = 0; i < phnum; ++i) {
+GElf_Phdr phdr_mem;
+phdr = gelf_getphdr(elf, i, &phdr_mem);
+if (phdr->p_type == PT_DYNAMIC) {
+ break;
+}
+ }
+ return phdr;
+}
+
+
+static size_t
+get_dyn_scnents (Elf *elf, GElf_Phdr * dyn_phdr)
+{
+ Elf_Data *data = elf_getdata_rawchunk(
+ elf, dyn_phdr->p_offset, dyn_phdr->p_filesz, ELF_T_DYN);
+ GElf_Dyn *dyn;
+ size_t dyn_idx = 0;
+ do
+ {
+GElf_Dyn dyn_mem;
+dyn = gelf_getdyn(data, dyn_idx, &dyn_mem);
+++dyn_idx;
+ } while (dyn->d_tag != DT_NULL);
+
+ return dyn_idx;
+}
+
+
static void
handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr)
{
int class = gelf_getclass (ebl->elf);
+ GElf_Phdr *dyn_phdr;
GElf_Shdr glink_mem;
GElf_Shdr *glink;
Elf_Data *data;
size_t cnt;
size_t shstrndx;
- size_t sh_entsize;
+ size_t dyn_scnents;
+
+ /* Get the dynamic section entry number */
+ dyn_phdr = get_dyn_phdr (ebl->elf);
+ dyn_scnents = get_dyn_scnents (ebl->elf, dyn_phdr);
/* Get the data of the section. */
data = elf_getdata (scn, NULL);
@@ -1802,8 +1841,6 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr
*shdr)
error (EXIT_FAILURE, 0,
_("cannot get section header string table index"));
- sh_entsize = gelf_fsize (ebl->elf, ELF_T_DYN, 1, EV_CURRENT);
-
glink = gelf_getshdr (elf_getscn (ebl->elf, shdr->sh_link), &glink_mem);
if (glink == NULL)
error (EXIT_FAILURE, 0, _("invalid sh_link value in section %zu"),
@@ -1813,15 +1850,15 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr
*shdr)
\nDynamic segment contains %lu entry:\n Addr: %#0*" PRIx64 " Offset:
%#08" PRIx64 " Link to section: [%2u] '%s'\n",
"\
\nDynamic segment contains %lu entries:\n Addr: %#0*" PRIx64 " Offset:
%#08" PRIx64 " Link to section: [%2u] '%s'\n",
-shdr->sh_size / sh_entsize),
- (unsigned long int) (shdr->sh_size / sh_entsize),
+dyn_scnents),
+ (unsigned long int) dyn_scnents,
class == ELFCLASS32 ? 10 : 18, shdr->sh_addr,
shdr->sh_offset,
(int) shdr->sh_link,
elf_strptr (ebl->elf, shstrndx, glink->sh_name));
fputs_unlocked (_(" Type Value\n"), stdout);
- for (cnt = 0; cnt < shdr->sh_size / sh_entsize; ++cnt)
+ for (cnt = 0; cnt < dyn_scnents; ++cnt)
{
GElf_Dyn dynmem;
GElf_Dyn *dyn = gelf_getdyn (data, cnt, &dynmem);
diff --git a/tests/alldts.c b/tests/alldts.c
index 3e9f9fe6..d0fe4f24 100644
--- a/tests/alldts.c
+++ b/tests/alldts.c
@@ -44,7 +44,7 @@ main (void)
Dwelf_Strent *shstrtabse;
const Elf32_Sword dtflags[] =
{
- DT_NULL, DT_NEEDED, DT_PLTRELSZ, DT_PLTGOT,
+ DT_NEEDED, DT_PLTRELSZ, DT_PLTGOT,
DT_HASH, DT_STRTAB, DT_SYMTAB, DT_RELA,
DT_RELASZ, DT_RELAENT, DT_STRSZ, DT_SYMENT,
DT_INIT, DT_FINI, DT_SONAME, DT_RPATH,
@@ -61,7 +61,8 @@ main (void)
DT_GNU_LIBLIST, DT_CONFIG, DT_DEPAUDIT, DT_AUDIT,
DT_PLTPAD, DT_MOVETAB, DT_SYMINFO, DT_RELACOUNT,
DT_RELCOUNT, DT_FLAGS_1, DT_VERDEF, DT_VERDEFNUM,
- DT_VERNEED, DT_VERNEEDNUM, DT_AUXILIARY, DT_FILTER
+ DT_VERNEED, DT_VERNEEDNUM, DT_AUXILIARY, DT_FILTER,
+ DT_NULL
};
const int ndtflags = sizeof (dtflags) / sizeof (dtflags[0]);
diff --git a/tests/run-alldts.sh b/tests/run-alldts.sh
index 6a9a9ece..ce3630b0 100755
--- a/tests/run-alldts.sh
+++ b/tests/run-alldts.sh
@@ -27,7 +27,6 @@ testrun_compare ${abs_top_builddir}/src/readelf -d
testfile-alldts <<\EOF
Dynamic segment contains 66 entries:
Addr: 0x01a0 Offset: 0x78 Link to section:
Re: Proposing elfutils-0.187 for Monday 25 April
Hey Mark, The latest patch for PR28928 locates in bugzilla, and it's attached in https://sourceware.org/bugzilla/show_bug.cgi?id=28928 Di On Fri, Apr 15, 2022 at 6:41 PM Mark Wielaard wrote: > Hi, > > It has been 5 months since elfutils 0.186 was released. There have been > more than 70 commits since then. And 20 bugs closed. > > I propose we do an elfutils 0.187 release Monday 25 April. > > There are still a couple of unreviewed patches: > https://patchwork.sourceware.org/project/elfutils/list/ > I'll try to go over them all before the release. > > If there are any other issues left, please let me know. > > Lets try to do the next release in 3 months, so there isn't such a big > gap between releases. > > Cheers, > > Mark > >
[PATCH] readelf: Support --dynamic with --use-dynamic
>From 3ac23c2584d76114deab0c0af6f4af99068dc7f4 Mon Sep 17 00:00:00 2001
From: Di Chen
Date: Thu, 28 Apr 2022 19:55:33 +0800
Subject: [PATCH] readelf: Support --dynamic with --use-dynamic
Currently, eu-readelf is using section headers to dump the dynamic
segment information (print_dynamic -> handle_dynamic).
This patch adds new options to eu-readelf (-D, --use-dynamic)
for (-d, --dynamic).
https://sourceware.org/bugzilla/show_bug.cgi?id=28873
Signed-off-by: Di Chen
---
src/readelf.c | 168 +---
tests/Makefile.am | 3 +-
tests/run-readelf-Dd.sh | 65
3 files changed, 223 insertions(+), 13 deletions(-)
create mode 100755 tests/run-readelf-Dd.sh
diff --git a/src/readelf.c b/src/readelf.c
index 4b6aab2b..e578456b 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -137,6 +137,8 @@ static const struct argp_option options[] =
{ "string-dump", 'p', NULL, OPTION_ALIAS | OPTION_HIDDEN, NULL, 0 },
{ "archive-index", 'c', NULL, 0,
N_("Display the symbol index of an archive"), 0 },
+ { "use-dynamic", 'D', NULL, 0,
+N_("Use the dynamic section info when displaying symbols"), 0 },
{ NULL, 0, NULL, 0, N_("Output control:"), 0 },
{ "numeric-addresses", 'N', NULL, 0,
@@ -195,6 +197,9 @@ static bool print_symbol_table;
/* True if (only) the dynsym table should be printed. */
static bool print_dynsym_table;
+/* True if reconstruct dynamic symbol table from the PT_DYNAMIC segment.
*/
+static bool use_dynamic_segment;
+
/* A specific section name, or NULL to print all symbol tables. */
static char *symbol_table_section;
@@ -268,6 +273,19 @@ static enum section_e
| section_macro | section_addr | section_types)
} print_debug_sections, implicit_debug_sections;
+enum dyn_idx
+{
+ i_strsz,
+ i_verneed,
+ i_verdef,
+ i_versym,
+ i_symtab,
+ i_strtab,
+ i_hash,
+ i_gnu_hash,
+ i_max
+};
+
/* Select hex dumping of sections. */
static struct section_argument *dump_data_sections;
static struct section_argument **dump_data_sections_tail =
&dump_data_sections;
@@ -318,6 +336,11 @@ static void dump_strings (Ebl *ebl);
static void print_strings (Ebl *ebl);
static void dump_archive_index (Elf *, const char *);
+/* Declarations of local functions for use-dynamic. */
+static Elf_Data * get_dynscn_strtab(Elf *elf, GElf_Phdr *phdr);
+static void get_dynscn_addrs(Elf *elf, GElf_Phdr *phdr, GElf_Addr
addrs[i_max]);
+static void find_offsets(Elf *elf, GElf_Addr main_bias, size_t n,
+GElf_Addr addrs[n], GElf_Off offs[n]);
/* Looked up once with gettext in main. */
static char *yes_str;
@@ -429,6 +452,9 @@ parse_opt (int key, char *arg,
print_dynamic_table = true;
any_control_option = true;
break;
+case 'D':
+ use_dynamic_segment = true;
+ break;
case 'e':
print_debug_sections |= section_exception;
any_control_option = true;
@@ -1791,7 +1817,7 @@ get_dyn_ents (Elf_Data * dyn_data)
static void
-handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr)
+handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr, GElf_Phdr *phdr)
{
int class = gelf_getclass (ebl->elf);
GElf_Shdr glink_mem;
@@ -1802,13 +1828,20 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr
*shdr)
size_t dyn_ents;
/* Get the data of the section. */
- data = elf_getdata (scn, NULL);
+ if (use_dynamic_segment)
+data = elf_getdata_rawchunk(
+ ebl->elf, phdr->p_offset, phdr->p_filesz, ELF_T_DYN);
+ else
+data = elf_getdata (scn, NULL);
+
if (data == NULL)
return;
/* Get the dynamic section entry number */
dyn_ents = get_dyn_ents (data);
+ if (!use_dynamic_segment)
+{
/* Get the section header string table index. */
if (unlikely (elf_getshdrstrndx (ebl->elf, &shstrndx) < 0))
error_exit (0, _("cannot get section header string table index"));
@@ -1828,8 +1861,25 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr
*shdr)
shdr->sh_offset,
(int) shdr->sh_link,
elf_strptr (ebl->elf, shstrndx, glink->sh_name));
+} else {
+ printf (ngettext ("\
+\nDynamic segment contains %lu entry:\n Addr: %#0*" PRIx64 " Offset:
%#08" PRIx64 "\n",
+"\
+\nDynamic segment contains %lu entries:\n Addr: %#0*" PRIx64 " Offset:
%#08" PRIx64 "\n",
+dyn_ents),
+ (unsigned long int) dyn_ents,
+ class == ELFCLASS32 ? 10 : 18, phdr->p_paddr,
+ phdr->p_offset);
+}
+
fputs_unlocked (_(" Type Value\n"), stdout);
+ /* if --use-dynamic option is enabled,
+ use the string table to get the related library info. */
+ Elf_Data *strtab_data = NULL;
+ if (use_dynamic_segment)
+strtab_data = get_dynscn_strtab(ebl->elf, phdr);
+
for (cnt = 0; cnt < dyn_ents; ++cnt)
{
GElf_Dyn dynmem;
@@ -1841,6 +1891,12 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr
*shdr)
printf (" %-17s ",
ebl_dynamic_tag_name (ebl, dyn->d_tag, buf, sizeof (buf)));
+ char *lib_name = NULL;
+ if (use_dynamic_seg
Re: [PATCH] readelf: Support --dynamic with --use-dynamic
Thanks Mark,
All the request changes are fixed, ready for review again.
1. help message updated: "Use the dynamic segment when possible for
displaying info"
2. move enum dyn_idx to a proper place
3. add strtab_data's NULL check in function: handle_dynamic()
4. add phdr's NULL check in function: print_dynamic()
5. add comments for function: find_offsets()
6. remove redundant return-statement in function: get_dynscn_addrs()
7. add run-readelf-Dd.sh to EXTRA_DISTS
8. check strsz in (dyn->d_un.d_ptr < strtab_data->d_size) in function:
handle_dynamic()
On Fri, May 20, 2022 at 8:41 AM Mark Wielaard wrote:
> Hi,
>
> On Thu, May 05, 2022 at 09:01:24PM +0800, Di Chen via Elfutils-devel wrote:
> > From 3ac23c2584d76114deab0c0af6f4af99068dc7f4 Mon Sep 17 00:00:00 2001
> > From: Di Chen
> > Date: Thu, 28 Apr 2022 19:55:33 +0800
> > Subject: [PATCH] readelf: Support --dynamic with --use-dynamic
> >
> > Currently, eu-readelf is using section headers to dump the dynamic
> > segment information (print_dynamic -> handle_dynamic).
> >
> > This patch adds new options to eu-readelf (-D, --use-dynamic)
> > for (-d, --dynamic).
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=28873
>
> This looks pretty good. But there are lots of white-space issues which
> make it hard to apply the patch. Could you sent it again preferrably
> using git send-email ?
>
> > Signed-off-by: Di Chen
> > ---
> > src/readelf.c | 168 +---
> > tests/Makefile.am | 3 +-
> > tests/run-readelf-Dd.sh | 65
> > 3 files changed, 223 insertions(+), 13 deletions(-)
> > create mode 100755 tests/run-readelf-Dd.sh
> >
> > diff --git a/src/readelf.c b/src/readelf.c
> > index 4b6aab2b..e578456b 100644
> > --- a/src/readelf.c
> > +++ b/src/readelf.c
> > @@ -137,6 +137,8 @@ static const struct argp_option options[] =
> >{ "string-dump", 'p', NULL, OPTION_ALIAS | OPTION_HIDDEN, NULL, 0 },
> >{ "archive-index", 'c', NULL, 0,
> > N_("Display the symbol index of an archive"), 0 },
> > + { "use-dynamic", 'D', NULL, 0,
> > +N_("Use the dynamic section info when displaying symbols"), 0 },
>
> This doesn't handle symbols yet. Should it simply say:
> "Use the dynamic segment when possible for displaying info"
>
> >{ NULL, 0, NULL, 0, N_("Output control:"), 0 },
> >{ "numeric-addresses", 'N', NULL, 0,
> > @@ -195,6 +197,9 @@ static bool print_symbol_table;
> > /* True if (only) the dynsym table should be printed. */
> > static bool print_dynsym_table;
> >
> > +/* True if reconstruct dynamic symbol table from the PT_DYNAMIC segment.
> > */
> > +static bool use_dynamic_segment;
> > +
> > /* A specific section name, or NULL to print all symbol tables. */
> > static char *symbol_table_section;
> >
> > @@ -268,6 +273,19 @@ static enum section_e
> > | section_macro | section_addr | section_types)
> > } print_debug_sections, implicit_debug_sections;
> >
> > +enum dyn_idx
> > +{
> > + i_strsz,
> > + i_verneed,
> > + i_verdef,
> > + i_versym,
> > + i_symtab,
> > + i_strtab,
> > + i_hash,
> > + i_gnu_hash,
> > + i_max
> > +};
>
> Maybe move this just before it is used in declarations of the
> get_dyn... functions?
>
> > /* Select hex dumping of sections. */
> > static struct section_argument *dump_data_sections;
> > static struct section_argument **dump_data_sections_tail =
> > &dump_data_sections;
> > @@ -318,6 +336,11 @@ static void dump_strings (Ebl *ebl);
> > static void print_strings (Ebl *ebl);
> > static void dump_archive_index (Elf *, const char *);
> >
> > +/* Declarations of local functions for use-dynamic. */
> > +static Elf_Data * get_dynscn_strtab(Elf *elf, GElf_Phdr *phdr);
> > +static void get_dynscn_addrs(Elf *elf, GElf_Phdr *phdr, GElf_Addr
> > addrs[i_max]);
> > +static void find_offsets(Elf *elf, GElf_Addr main_bias, size_t n,
> > +GElf_Addr addrs[n], GElf_Off offs[n]);
>
> I mean, just before these.
>
> > /* Looked up once with gettext in main. */
> > static char *yes_str;
> > @@ -429,6 +452,9 @@ parse_opt (int key, char *arg,
> >print_dynamic_table = true;
> >any_control_option = true;
> >break;
> > +case 'D':
> > + use_dynamic_segment = true;
> > + break;
>
[PATCH] libdw/libdwfl: Add API for accessing registers
>From 8325b5311ff5618a7a66e5398652e2177cc53e78 Mon Sep 17 00:00:00 2001
From: Di Chen
Date: Tue, 19 Jul 2022 14:54:45 +0800
Subject: [PATCH] libdw/libdwfl: Add API for accessing registers
Dwfl has most of the infrastructure to keep the full unwind state,
including the state of unwound registers per frame using
Dwfl_Thread_Callbacks. But there is no public API to access the state,
except for the PC (dwfl_frame_pc).
This update renames previous state_get_reg() (in libdwfl/frame_unwind.c)
to dwfl_frame_reg(), adds a regno check, and makes it a public API.
Signed-off-by: Di Chen
---
libdw/libdw.map | 1 +
libdwfl/dwfl_frame_regs.c | 18 ++
libdwfl/frame_unwind.c| 21 +
libdwfl/libdwfl.h | 4
libdwfl/libdwflP.h| 2 ++
5 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/libdw/libdw.map b/libdw/libdw.map
index 6da25561..8f393438 100644
--- a/libdw/libdw.map
+++ b/libdw/libdw.map
@@ -370,4 +370,5 @@ ELFUTILS_0.186 {
ELFUTILS_0.188 {
global:
dwfl_get_debuginfod_client;
+dwfl_frame_reg;
} ELFUTILS_0.186;
diff --git a/libdwfl/dwfl_frame_regs.c b/libdwfl/dwfl_frame_regs.c
index 83b1abef..92c4a692 100644
--- a/libdwfl/dwfl_frame_regs.c
+++ b/libdwfl/dwfl_frame_regs.c
@@ -59,3 +59,21 @@ dwfl_thread_state_register_pc (Dwfl_Thread *thread,
Dwarf_Word pc)
state->pc_state = DWFL_FRAME_STATE_PC_SET;
}
INTDEF(dwfl_thread_state_register_pc)
+
+bool
+dwfl_frame_reg (Dwfl_Frame *state, unsigned regno, Dwarf_Word *val)
+{
+ if ((state->regs_set[regno / sizeof (*state->regs_set) / 8]
+ & ((uint64_t) 1U << (regno % (sizeof (*state->regs_set) * 8 ==
0)
+{
+ __libdwfl_seterrno (DWFL_E_INVALID_REGNO);
+ return false;
+}
+ if (! __libdwfl_frame_reg_get (state, regno, val))
+{
+ __libdwfl_seterrno (DWFL_E_INVALID_REGISTER);
+ return false;
+}
+ return true;
+}
+INTDEF(dwfl_frame_reg)
diff --git a/libdwfl/frame_unwind.c b/libdwfl/frame_unwind.c
index 9ac33833..38f5b332 100644
--- a/libdwfl/frame_unwind.c
+++ b/libdwfl/frame_unwind.c
@@ -78,17 +78,6 @@ __libdwfl_frame_reg_set (Dwfl_Frame *state, unsigned
regno, Dwarf_Addr val)
return true;
}
-static bool
-state_get_reg (Dwfl_Frame *state, unsigned regno, Dwarf_Addr *val)
-{
- if (! __libdwfl_frame_reg_get (state, regno, val))
-{
- __libdwfl_seterrno (DWFL_E_INVALID_REGISTER);
- return false;
-}
- return true;
-}
-
static int
bra_compar (const void *key_voidp, const void *elem_voidp)
{
@@ -211,7 +200,7 @@ expr_eval (Dwfl_Frame *state, Dwarf_Frame *frame, const
Dwarf_Op *ops,
}
break;
case DW_OP_reg0 ... DW_OP_reg31:
- if (! state_get_reg (state, op->atom - DW_OP_reg0, &val1)
+ if (! dwfl_frame_reg (state, op->atom - DW_OP_reg0, &val1)
|| ! push (val1))
{
free (stack.addrs);
@@ -219,14 +208,14 @@ expr_eval (Dwfl_Frame *state, Dwarf_Frame *frame,
const Dwarf_Op *ops,
}
break;
case DW_OP_regx:
- if (! state_get_reg (state, op->number, &val1) || ! push (val1))
+ if (! dwfl_frame_reg (state, op->number, &val1) || ! push (val1))
{
free (stack.addrs);
return false;
}
break;
case DW_OP_breg0 ... DW_OP_breg31:
- if (! state_get_reg (state, op->atom - DW_OP_breg0, &val1))
+ if (! dwfl_frame_reg (state, op->atom - DW_OP_breg0, &val1))
{
free (stack.addrs);
return false;
@@ -239,7 +228,7 @@ expr_eval (Dwfl_Frame *state, Dwarf_Frame *frame, const
Dwarf_Op *ops,
}
break;
case DW_OP_bregx:
- if (! state_get_reg (state, op->number, &val1))
+ if (! dwfl_frame_reg (state, op->number, &val1))
{
free (stack.addrs);
return false;
@@ -591,7 +580,7 @@ handle_cfi (Dwfl_Frame *state, Dwarf_Addr pc, Dwarf_CFI
*cfi, Dwarf_Addr bias)
else if (reg_ops == NULL)
{
/* REGNO is same-value. */
- if (! state_get_reg (state, regno, ®val))
+ if (! dwfl_frame_reg (state, regno, ®val))
continue;
}
else
diff --git a/libdwfl/libdwfl.h b/libdwfl/libdwfl.h
index b323e8fb..7c0dd87d 100644
--- a/libdwfl/libdwfl.h
+++ b/libdwfl/libdwfl.h
@@ -798,6 +798,10 @@ int dwfl_getthread_frames (Dwfl *dwfl, pid_t tid,
bool dwfl_frame_pc (Dwfl_Frame *state, Dwarf_Addr *pc, bool *isactivation)
__nonnull_attribute__ (1, 2);
+/* Return *val (register value) for frame STATE. */
+bool dwfl_frame_reg (Dwfl_Frame *state, unsigned regno, Dwarf_Addr *val)
+ __nonnull_attribute__ (1);
+
/* Return the internal debuginfod-client connection handle for the DWFL
session.
When the client connection has not yet been initialized, it will be
done on the
first call to this function. If elfutils is compiled without support
for debuginfod,
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index 9f598370..a6d4396a 100644
--- a/libdwfl/libdwflP.h
+++ b/libdwfl/libdwflP.h
@@ -81,6 +81,7 @@ typedef struct Dwfl_Process Dwfl_Process;
DWFL_ERROR (LIBEBL_BAD, N_("Internal error due to ebl")) \
DWFL_ERROR
[PATCH v2] libdw/libdwfl: Add API for accessing registers
>From 9c25b08e46c2031b569a85f91713d009b83f4c26 Mon Sep 17 00:00:00 2001
From: Di Chen
Date: Tue, 19 Jul 2022 14:54:45 +0800
Subject: [PATCH] libdw/libdwfl: Add API for accessing registers
Dwfl has most of the infrastructure to keep the full unwind state,
including the state of unwound registers per frame using
Dwfl_Thread_Callbacks. But there is no public API to access the state,
except for the PC (dwfl_frame_pc).
This update renames previous state_get_reg() (in libdwfl/frame_unwind.c)
to dwfl_frame_reg(), adds a regno check, and makes it a public API.
Signed-off-by: Di Chen
---
libdw/libdw.map | 1 +
libdwfl/dwfl_frame_regs.c | 18 ++
libdwfl/frame_unwind.c| 21 +
libdwfl/libdwfl.h | 4
libdwfl/libdwflP.h| 2 ++
5 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/libdw/libdw.map b/libdw/libdw.map
index 6da25561..8f393438 100644
--- a/libdw/libdw.map
+++ b/libdw/libdw.map
@@ -370,4 +370,5 @@ ELFUTILS_0.186 {
ELFUTILS_0.188 {
global:
dwfl_get_debuginfod_client;
+dwfl_frame_reg;
} ELFUTILS_0.186;
diff --git a/libdwfl/dwfl_frame_regs.c b/libdwfl/dwfl_frame_regs.c
index 83b1abef..92c4a692 100644
--- a/libdwfl/dwfl_frame_regs.c
+++ b/libdwfl/dwfl_frame_regs.c
@@ -59,3 +59,21 @@ dwfl_thread_state_register_pc (Dwfl_Thread *thread,
Dwarf_Word pc)
state->pc_state = DWFL_FRAME_STATE_PC_SET;
}
INTDEF(dwfl_thread_state_register_pc)
+
+bool
+dwfl_frame_reg (Dwfl_Frame *state, unsigned regno, Dwarf_Word *val)
+{
+ if ((state->regs_set[regno / sizeof (*state->regs_set) / 8]
+ & ((uint64_t) 1U << (regno % (sizeof (*state->regs_set) * 8 ==
0)
+{
+ __libdwfl_seterrno (DWFL_E_INVALID_REGNO);
+ return false;
+}
+ if (! __libdwfl_frame_reg_get (state, regno, val))
+{
+ __libdwfl_seterrno (DWFL_E_INVALID_REGISTER);
+ return false;
+}
+ return true;
+}
+INTDEF(dwfl_frame_reg)
diff --git a/libdwfl/frame_unwind.c b/libdwfl/frame_unwind.c
index 9ac33833..38f5b332 100644
--- a/libdwfl/frame_unwind.c
+++ b/libdwfl/frame_unwind.c
@@ -78,17 +78,6 @@ __libdwfl_frame_reg_set (Dwfl_Frame *state, unsigned
regno, Dwarf_Addr val)
return true;
}
-static bool
-state_get_reg (Dwfl_Frame *state, unsigned regno, Dwarf_Addr *val)
-{
- if (! __libdwfl_frame_reg_get (state, regno, val))
-{
- __libdwfl_seterrno (DWFL_E_INVALID_REGISTER);
- return false;
-}
- return true;
-}
-
static int
bra_compar (const void *key_voidp, const void *elem_voidp)
{
@@ -211,7 +200,7 @@ expr_eval (Dwfl_Frame *state, Dwarf_Frame *frame, const
Dwarf_Op *ops,
}
break;
case DW_OP_reg0 ... DW_OP_reg31:
- if (! state_get_reg (state, op->atom - DW_OP_reg0, &val1)
+ if (! dwfl_frame_reg (state, op->atom - DW_OP_reg0, &val1)
|| ! push (val1))
{
free (stack.addrs);
@@ -219,14 +208,14 @@ expr_eval (Dwfl_Frame *state, Dwarf_Frame *frame,
const Dwarf_Op *ops,
}
break;
case DW_OP_regx:
- if (! state_get_reg (state, op->number, &val1) || ! push (val1))
+ if (! dwfl_frame_reg (state, op->number, &val1) || ! push (val1))
{
free (stack.addrs);
return false;
}
break;
case DW_OP_breg0 ... DW_OP_breg31:
- if (! state_get_reg (state, op->atom - DW_OP_breg0, &val1))
+ if (! dwfl_frame_reg (state, op->atom - DW_OP_breg0, &val1))
{
free (stack.addrs);
return false;
@@ -239,7 +228,7 @@ expr_eval (Dwfl_Frame *state, Dwarf_Frame *frame, const
Dwarf_Op *ops,
}
break;
case DW_OP_bregx:
- if (! state_get_reg (state, op->number, &val1))
+ if (! dwfl_frame_reg (state, op->number, &val1))
{
free (stack.addrs);
return false;
@@ -591,7 +580,7 @@ handle_cfi (Dwfl_Frame *state, Dwarf_Addr pc, Dwarf_CFI
*cfi, Dwarf_Addr bias)
else if (reg_ops == NULL)
{
/* REGNO is same-value. */
- if (! state_get_reg (state, regno, ®val))
+ if (! dwfl_frame_reg (state, regno, ®val))
continue;
}
else
diff --git a/libdwfl/libdwfl.h b/libdwfl/libdwfl.h
index b323e8fb..aba75afe 100644
--- a/libdwfl/libdwfl.h
+++ b/libdwfl/libdwfl.h
@@ -798,6 +798,10 @@ int dwfl_getthread_frames (Dwfl *dwfl, pid_t tid,
bool dwfl_frame_pc (Dwfl_Frame *state, Dwarf_Addr *pc, bool *isactivation)
__nonnull_attribute__ (1, 2);
+/* Return *val (register value) for frame STATE. */
+bool dwfl_frame_reg (Dwfl_Frame *state, unsigned regno, Dwarf_Word *val)
+ __nonnull_attribute__ (1);
+
/* Return the internal debuginfod-client connection handle for the DWFL
session.
When the client connection has not yet been initialized, it will be
done on the
first call to this function. If elfutils is compiled without support
for debuginfod,
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index 9f598370..a6d4396a 100644
--- a/libdwfl/libdwflP.h
+++ b/libdwfl/libdwflP.h
@@ -81,6 +81,7 @@ typedef struct Dwfl_Process Dwfl_Process;
DWFL_ERROR (LIBEBL_BAD, N_("Internal error due to ebl")) \
DWFL_ERROR
Re: [PATCH v2] libdw/libdwfl: Add API for accessing registers
Thanks for the review, Mark.
Re-pushed a patch for it.
1. NEWS entry added.
2. __libdwfl_frame_reg_get/dwfl_frame_reg updated with return int.
3. New DWFL_ERROR added: REGISTER_VAL_UNKNOWN.
On Thu, Jul 21, 2022 at 10:32 PM Mark Wielaard wrote:
> Hi,
>
> On Tue, Jul 19, 2022 at 10:21:21PM +0800, Di Chen via Elfutils-devel wrote:
> > From 9c25b08e46c2031b569a85f91713d009b83f4c26 Mon Sep 17 00:00:00 2001
> > From: Di Chen
> > Date: Tue, 19 Jul 2022 14:54:45 +0800
> > Subject: [PATCH] libdw/libdwfl: Add API for accessing registers
> >
> > Dwfl has most of the infrastructure to keep the full unwind state,
> > including the state of unwound registers per frame using
> > Dwfl_Thread_Callbacks. But there is no public API to access the state,
> > except for the PC (dwfl_frame_pc).
> >
> > This update renames previous state_get_reg() (in libdwfl/frame_unwind.c)
> > to dwfl_frame_reg(), adds a regno check, and makes it a public API.
>
> This looks mostly fine, some comments below. But it is hard to apply
> since your mailer seems to break up lines. If you cannot use git
> send-email to send patches it might be easier to use git format-patch
> and attach the result. Or to use the sourcehut mirror to create a
> fork, push your patches there and let sourcehut sent the patches as
> email: https://git.sr.ht/~sourceware/elfutils
>
> > Signed-off-by: Di Chen
> > ---
> > libdw/libdw.map | 1 +
> > libdwfl/dwfl_frame_regs.c | 18 ++
> > libdwfl/frame_unwind.c| 21 +
> > libdwfl/libdwfl.h | 4
> > libdwfl/libdwflP.h| 2 ++
> > 5 files changed, 30 insertions(+), 16 deletions(-)
> >
> > diff --git a/libdw/libdw.map b/libdw/libdw.map
> > index 6da25561..8f393438 100644
> > --- a/libdw/libdw.map
> > +++ b/libdw/libdw.map
> > @@ -370,4 +370,5 @@ ELFUTILS_0.186 {
> > ELFUTILS_0.188 {
> >global:
> > dwfl_get_debuginfod_client;
> > +dwfl_frame_reg;
> > } ELFUTILS_0.186;
>
> Good. Could you also add a NEWS entry for the new public function?
>
> > diff --git a/libdwfl/dwfl_frame_regs.c b/libdwfl/dwfl_frame_regs.c
> > index 83b1abef..92c4a692 100644
> > --- a/libdwfl/dwfl_frame_regs.c
> > +++ b/libdwfl/dwfl_frame_regs.c
> > @@ -59,3 +59,21 @@ dwfl_thread_state_register_pc (Dwfl_Thread *thread,
> > Dwarf_Word pc)
> >state->pc_state = DWFL_FRAME_STATE_PC_SET;
> > }
> > INTDEF(dwfl_thread_state_register_pc)
> > +
> > +bool
> > +dwfl_frame_reg (Dwfl_Frame *state, unsigned regno, Dwarf_Word *val)
> > +{
> > + if ((state->regs_set[regno / sizeof (*state->regs_set) / 8]
> > + & ((uint64_t) 1U << (regno % (sizeof (*state->regs_set) * 8
> ==
> > 0)
> > +{
> > + __libdwfl_seterrno (DWFL_E_INVALID_REGNO);
> > + return false;
> > +}
> > + if (! __libdwfl_frame_reg_get (state, regno, val))
> > +{
> > + __libdwfl_seterrno (DWFL_E_INVALID_REGISTER);
> > + return false;
> > +}
> > + return true;
> > +}
>
> I see what you want to do here. First check if the register is
> actually saved in the frame, if not return an DWFL_E_INVALID_REGNO. If
> it is set, then get the value and return it (or if that fails, return
> a different error DWFL_E_INVALID_REGISTER).
>
> But dwfl_frame_reg takes a DWARF register number, but the regs_set
> array uses an internal numbering. You first have to translate the
> numbering using ebl_dwarf_to_regno.
>
> Also we might want to give a better error than DWFL_E_INVALID_REGNO
> when the register is simply unknown. DWFL_E_REGISTER_VAL_UNKNOWN
> maybe?
>
> The real problem is that __libdwfl_frame_reg_get returns a boolean,
> true if things worked fine and the register value is known, false if
> the register value is unknown or the DWARF register number was invalid
> or some other error occured.
>
> It might be an idea to change the interface of __libdwfl_frame_reg_get
> (and dwfl_frame_reg) to return an int. With zero as success. -1 for an
> error (invalid register number) and 1 for a valid DWARF register
> value, but the register value for the given frame is
> unknown. __libdwfl_frame_reg_get could then also call
> __libdwfl_seterrno itself.
>
> This does need a few more code changes though. Changing all calls to
> __libdwfl_frame_reg_get with if (__libdwfl_frame_reg_get (state,
> regno, &val) == 0). And remove any redundant __libdwfl_seterrno calls.
>
> > +INTDEF(dwfl_frame_reg)
>
> Good. Now you can use INTUSE (dwfl_frame_reg) for internal calls.
&
Re: [PATCH v2] libdw/libdwfl: Add API for accessing registers
Re-pushed for fixing run-backtrace-core-sparc.sh failure which resulted
from some wrong register number.
On Fri, Jul 29, 2022 at 9:27 PM Di Chen wrote:
> Thanks for the review, Mark.
>
> Re-pushed a patch for it.
>
> 1. NEWS entry added.
> 2. __libdwfl_frame_reg_get/dwfl_frame_reg updated with return int.
> 3. New DWFL_ERROR added: REGISTER_VAL_UNKNOWN.
>
>
> On Thu, Jul 21, 2022 at 10:32 PM Mark Wielaard wrote:
>
>> Hi,
>>
>> On Tue, Jul 19, 2022 at 10:21:21PM +0800, Di Chen via Elfutils-devel
>> wrote:
>> > From 9c25b08e46c2031b569a85f91713d009b83f4c26 Mon Sep 17 00:00:00 2001
>> > From: Di Chen
>> > Date: Tue, 19 Jul 2022 14:54:45 +0800
>> > Subject: [PATCH] libdw/libdwfl: Add API for accessing registers
>> >
>> > Dwfl has most of the infrastructure to keep the full unwind state,
>> > including the state of unwound registers per frame using
>> > Dwfl_Thread_Callbacks. But there is no public API to access the state,
>> > except for the PC (dwfl_frame_pc).
>> >
>> > This update renames previous state_get_reg() (in libdwfl/frame_unwind.c)
>> > to dwfl_frame_reg(), adds a regno check, and makes it a public API.
>>
>> This looks mostly fine, some comments below. But it is hard to apply
>> since your mailer seems to break up lines. If you cannot use git
>> send-email to send patches it might be easier to use git format-patch
>> and attach the result. Or to use the sourcehut mirror to create a
>> fork, push your patches there and let sourcehut sent the patches as
>> email: https://git.sr.ht/~sourceware/elfutils
>>
>> > Signed-off-by: Di Chen
>> > ---
>> > libdw/libdw.map | 1 +
>> > libdwfl/dwfl_frame_regs.c | 18 ++
>> > libdwfl/frame_unwind.c| 21 +
>> > libdwfl/libdwfl.h | 4
>> > libdwfl/libdwflP.h| 2 ++
>> > 5 files changed, 30 insertions(+), 16 deletions(-)
>> >
>> > diff --git a/libdw/libdw.map b/libdw/libdw.map
>> > index 6da25561..8f393438 100644
>> > --- a/libdw/libdw.map
>> > +++ b/libdw/libdw.map
>> > @@ -370,4 +370,5 @@ ELFUTILS_0.186 {
>> > ELFUTILS_0.188 {
>> >global:
>> > dwfl_get_debuginfod_client;
>> > +dwfl_frame_reg;
>> > } ELFUTILS_0.186;
>>
>> Good. Could you also add a NEWS entry for the new public function?
>>
>> > diff --git a/libdwfl/dwfl_frame_regs.c b/libdwfl/dwfl_frame_regs.c
>> > index 83b1abef..92c4a692 100644
>> > --- a/libdwfl/dwfl_frame_regs.c
>> > +++ b/libdwfl/dwfl_frame_regs.c
>> > @@ -59,3 +59,21 @@ dwfl_thread_state_register_pc (Dwfl_Thread *thread,
>> > Dwarf_Word pc)
>> >state->pc_state = DWFL_FRAME_STATE_PC_SET;
>> > }
>> > INTDEF(dwfl_thread_state_register_pc)
>> > +
>> > +bool
>> > +dwfl_frame_reg (Dwfl_Frame *state, unsigned regno, Dwarf_Word *val)
>> > +{
>> > + if ((state->regs_set[regno / sizeof (*state->regs_set) / 8]
>> > + & ((uint64_t) 1U << (regno % (sizeof (*state->regs_set) * 8
>> ==
>> > 0)
>> > +{
>> > + __libdwfl_seterrno (DWFL_E_INVALID_REGNO);
>> > + return false;
>> > +}
>> > + if (! __libdwfl_frame_reg_get (state, regno, val))
>> > +{
>> > + __libdwfl_seterrno (DWFL_E_INVALID_REGISTER);
>> > + return false;
>> > +}
>> > + return true;
>> > +}
>>
>> I see what you want to do here. First check if the register is
>> actually saved in the frame, if not return an DWFL_E_INVALID_REGNO. If
>> it is set, then get the value and return it (or if that fails, return
>> a different error DWFL_E_INVALID_REGISTER).
>>
>> But dwfl_frame_reg takes a DWARF register number, but the regs_set
>> array uses an internal numbering. You first have to translate the
>> numbering using ebl_dwarf_to_regno.
>>
>> Also we might want to give a better error than DWFL_E_INVALID_REGNO
>> when the register is simply unknown. DWFL_E_REGISTER_VAL_UNKNOWN
>> maybe?
>>
>> The real problem is that __libdwfl_frame_reg_get returns a boolean,
>> true if things worked fine and the register value is known, false if
>> the register value is unknown or the DWARF register number was invalid
>> or some other error occured.
>>
>> It might be an idea to change the interface of __libdwfl_frame_reg_get
>> (and dwfl_frame_reg) to return an int. With zero as su
