Re: [PATCH] debuginfod: PR28242 - extend server http-response metrics
Hi, On Fri, 2021-10-01 at 22:05 +0800, Di Chen via Elfutils-devel wrote: > 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 BTW. If possible use git send-email HEAD^ or attach the result of git format-patch HEAD^ which will make sure some whitespace/linebreaks pass through the mailinglist better. > 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 I believe the patch does what it says, the locking seems good and the testcases have been adapted. Nice work. It is missing a ChangeLog entry which would have helped a little with review. But I would like someone else (Frank?) who knows more about the metrics to take a quick peek before pushing. Thanks, Mark
patchworks and sourcehut for elfutils
Hi, To make patch tracking slighly easier there is now a patchwork instance on sourceware that should show the status of all outstanding patches sent to the mailinglist: https://patchwork.sourceware.org/project/elfutils/list/ It is a bit experimental and doesn't really come with documentation yet. Also there is a mirror of the elfutils repo (and other sourceware git repos) on sourcehut: https://sr.ht/~sourceware/elfutils/ If you have a sourcehut account then you can easily fork the project source repos (the main one and the website one): https://git.sr.ht/~sourceware/elfutils https://git.sr.ht/~sourceware/elfutils-htdocs You can then clone your fork locally and create any branches and commits like you normally would. After pushing your changes back to sourcehut you can tell sourcehut to sent to patches to the mailinglist by using the "Prepare a patchset" button to submit patches to the project. sourcehut can generate the appropriate git send-email commands and/or sent the patches for you. Cheers, Mark
[Bug debuginfod/28430] New: debuginfod should support a read-only database mode
https://sourceware.org/bugzilla/show_bug.cgi?id=28430 Bug ID: 28430 Summary: debuginfod should support a read-only database mode Product: elfutils Version: unspecified Status: NEW Severity: normal Priority: P2 Component: debuginfod Assignee: unassigned at sourceware dot org Reporter: fche at redhat dot com CC: elfutils-devel at sourceware dot org Target Milestone: --- For purposes of scaling workers against a shared database, this sort of thing should work: debuginfod -d 'file:///PATH?mode=ro' -R -Z .. which would force read-only operation. (Presumably, the PATH represents a database that some other debuginfod is populating/grooming.) It could be enough to have a flag that suppresses startup-time DDL application, grooming, traversal, and scanning, leaving only the libmicrohttpd-related threads. -- You are receiving this mail because: You are on the CC list for the bug.
Re: patchworks and sourcehut for elfutils
Hi, On Wed, Oct 06, 2021 at 06:25:04PM +0200, Mark Wielaard wrote: > To make patch tracking slighly easier there is now a patchwork instance > on sourceware that should show the status of all outstanding patches > sent to the mailinglist: > https://patchwork.sourceware.org/project/elfutils/list/ > It is a bit experimental and doesn't really come with documentation > yet. Although you can work through the website, there is git integration through git-pw which allows to interact with the patchwork server through the command line. https://patchwork.readthedocs.io/projects/git-pw/ To setup git-pw you need to register on the website first: https://patchwork.sourceware.org/register/ Then login and generate an "api token" under user authentication: https://patchwork.sourceware.org/user/ To configure git pw in your local elfutils.git checkout: git config pw.server https://patchwork.sourceware.org/api/1.2/ git config pw.token super-secret-hex-token-string git config pw.project elfutils Or add this to you .git/config: [pw] server = https://patchwork.sourceware.org/api/1.2/ token = super-secret-hex-token-string project = elfutils Now you can easily inspect and interact with the patch queue: $ git pw patch list # list all pending patches $ git pw patch show 45831 # to show more info on a particular patch And then either git pw download ID or git pw apply ID and/or git pw update to work with the actual patch. More documentation at https://patchwork.readthedocs.io/projects/git-pw/ Cheers, Mark
[PATCH] libdw: Use signedness of subrange type to determine array bounds
When calculating the array size check if the subrange has an associate
type, if it does then check the type to determine whether the upper
and lower values need to be interpreted as signed of unsigned
values. We default to signed because that is what the testcase
run-aggregate-size.sh testfile-size4 expects (this is an hardwritten
testcase, we could have chosen a different default).
https://sourceware.org/bugzilla/show_bug.cgi?id=28294
Signed-off-by: Mark Wielaard
---
libdw/ChangeLog | 6 +
libdw/dwarf_aggregate_size.c | 44 +++-
2 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index b707bbfe..4275b830 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,9 @@
+2021-10-06 Mark Wielaard
+
+ * dwarf_aggregate_size.c (array_size): Check signedness of child DIE
+ type. Use dwarf_formsdata or dwarf_formudata to get the lower and
+ upper bounds.
+
2021-09-08 Mark Wielaard
* dwarf_begin_elf.c (valid_p): Identify ELF class and use this to set
diff --git a/libdw/dwarf_aggregate_size.c b/libdw/dwarf_aggregate_size.c
index 75105e4d..96023d69 100644
--- a/libdw/dwarf_aggregate_size.c
+++ b/libdw/dwarf_aggregate_size.c
@@ -83,19 +83,51 @@ array_size (Dwarf_Die *die, Dwarf_Word *size,
}
else
{
+ bool is_signed = true;
+ if (INTUSE(dwarf_attr) (get_type (&child, attr_mem, &type_mem),
+ DW_AT_encoding, attr_mem) != NULL)
+ {
+ Dwarf_Word encoding;
+ if (INTUSE(dwarf_formudata) (attr_mem, &encoding) == 0)
+ is_signed = (encoding == DW_ATE_signed
+|| encoding == DW_ATE_signed_char);
+ }
+
Dwarf_Sword upper;
Dwarf_Sword lower;
- if (INTUSE(dwarf_formsdata) (INTUSE(dwarf_attr_integrate)
- (&child, DW_AT_upper_bound,
- attr_mem), &upper) != 0)
- return -1;
+ if (is_signed)
+ {
+ if (INTUSE(dwarf_formsdata) (INTUSE(dwarf_attr_integrate)
+ (&child, DW_AT_upper_bound,
+ attr_mem), &upper) != 0)
+ return -1;
+ }
+ else
+ {
+ Dwarf_Word unsigned_upper;
+ if (INTUSE(dwarf_formudata) (INTUSE(dwarf_attr_integrate)
+ (&child, DW_AT_upper_bound,
+ attr_mem), &unsigned_upper) !=
0)
+ return -1;
+ upper = unsigned_upper;
+ }
/* Having DW_AT_lower_bound is optional. */
if (INTUSE(dwarf_attr_integrate) (&child, DW_AT_lower_bound,
attr_mem) != NULL)
{
- if (INTUSE(dwarf_formsdata) (attr_mem, &lower) != 0)
- return -1;
+ if (is_signed)
+ {
+ if (INTUSE(dwarf_formsdata) (attr_mem, &lower) != 0)
+ return -1;
+ }
+ else
+ {
+ Dwarf_Word unsigned_lower;
+ if (INTUSE(dwarf_formudata) (attr_mem, &unsigned_lower)
!= 0)
+ return -1;
+ lower = unsigned_lower;
+ }
}
else
{
--
2.32.0
[Bug libdw/28294] dwarf_aggregate_size fails on some array types
https://sourceware.org/bugzilla/show_bug.cgi?id=28294 --- Comment #3 from Mark Wielaard --- Patch posted: https://sourceware.org/pipermail/elfutils-devel/2021q4/004248.html -- You are receiving this mail because: You are on the CC list for the bug.
Re: [PATCH] debuginfod: PR28242 - extend server http-response metrics
Hi - > But I would like someone else (Frank?) who knows more about the metrics > to take a quick peek before pushing. Thanks, pushed with a couple of small tweaks. - FChE
[Bug debuginfod/28242] extend server http-response metrics with artifact-type content
https://sourceware.org/bugzilla/show_bug.cgi?id=28242 Frank Ch. Eigler changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #2 from Frank Ch. Eigler --- commit 260a3105cc0e378882110ba787cd58815183c454 (HEAD -> master, origin/master, origin/HEAD) Author: Di Chen Date: Wed Oct 6 17:04:08 2021 -0400 PR28242: debuginfod prometheus metric widening 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. v2, tweaked patch to perform artifact-type sanitization at point of vulnerability rather than in general metric tabulation logic. Signed-off-by: Di Chen Signed-off-by: Frank Ch. Eigler -- You are receiving this mail because: You are on the CC list for the bug.
Re: [patch] PR27783: default debuginfod-urls profile rework
Hi Frank,
On Sun, Oct 03, 2021 at 05:33:33PM -0400, Frank Ch. Eigler via Elfutils-devel
wrote:
> commit 0c634f243d266ce8841fd311433d5d79555fabf9
> Author: Frank Ch. Eigler
> Date: Sun Oct 3 17:04:24 2021 -0400
>
> PR27783: switch default debuginfod-urls to drop-in style files
>
> Rewrote and commented the /etc/profile.d csh and sh script fragments
> to take the default $DEBUGINFOD_URLS from the union of drop-in files:
> /etc/debuginfod/*.urls. Hand-tested with csh and bash, with
> conditions including no prior $DEBUGINFOD_URLS, nonexistent .urls
> files, multiple entries in .urls files.
>
> Signed-off-by: Frank Ch. Eigler
Could you add something about this to NEWS so packagers know how to
update to the new scheme?
> diff --git a/config/ChangeLog b/config/ChangeLog
> index b2c0af8ac816..bd41654f5492 100644
> --- a/config/ChangeLog
> +++ b/config/ChangeLog
> @@ -1,3 +1,11 @@
> +2021-10-03 Frank Ch. Eigler
> +
> + PR27783
> + * profile.sh, profile.csh: Rewrite to document and set DEBUGINFOD_URLS
> + from /etc/debuginfod/*.urls files.
> + * Makefile.am: Install the configury-specified .urls file.
> + * elfutils.spec: Package it.
> +
> 2021-09-05 Dmitry V. Levin
>
> * eu.am (STACK_USAGE_NO_ERROR): New variable.
> diff --git a/config/Makefile.am b/config/Makefile.am
> index a66f54904991..0d3ba164ee3a 100644
> --- a/config/Makefile.am
> +++ b/config/Makefile.am
> @@ -40,9 +40,16 @@ pkgconfig_DATA += libdebuginfod.pc
> install-data-local:
> $(INSTALL_DATA) profile.sh -D
> $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.sh
> $(INSTALL_DATA) profile.csh -D
> $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.csh
> + mkdir -p $(DESTDIR)$(sysconfdir)/debuginfod
> + if [ -n "@DEBUGINFOD_URLS@" ]; then \
> + echo "@DEBUGINFOD_URLS@" >
> $(DESTDIR)$(sysconfdir)/debuginfod/elfutils.urls; \
> + fi
>
> uninstall-local:
> - rm -f $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.sh
> $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.csh
> + rm -f $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.sh
> + rm -f $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.csh
> + rm -f $(DESTDIR)$(sysconfdir)/debuginfod/elfutils.urls
> + -rmdir $(DESTDIR)$(sysconfdir)/debuginfod
> endif
>
> if MAINTAINER_MODE
> diff --git a/config/elfutils.spec.in b/config/elfutils.spec.in
> index 043762653c90..8f6a8e03202f 100644
> --- a/config/elfutils.spec.in
> +++ b/config/elfutils.spec.in
> @@ -301,6 +301,7 @@ fi
> %{_mandir}/man1/debuginfod-find.1*
> %{_mandir}/man7/debuginfod*.7*
> %config(noreplace) %{_sysconfdir}/profile.d/*
> +%config(noreplace) %{_sysconfdir}/debuginfod/*
>
> %files debuginfod-client-devel
> %defattr(-,root,root)
> diff --git a/config/profile.csh.in b/config/profile.csh.in
> index 0a2d6d162019..29e59a709450 100644
> --- a/config/profile.csh.in
> +++ b/config/profile.csh.in
> @@ -1,11 +1,16 @@
> -if ("@DEBUGINFOD_URLS@" != "") then
> - if ($?DEBUGINFOD_URLS) then
> - if ($%DEBUGINFOD_URLS) then
> - setenv DEBUGINFOD_URLS "$DEBUGINFOD_URLS
> @DEBUGINFOD_URLS@"
> - else
> - setenv DEBUGINFOD_URLS "@DEBUGINFOD_URLS@"
> - endif
> - else
> - setenv DEBUGINFOD_URLS "@DEBUGINFOD_URLS@"
> - endif
> +
> +# $HOME/.login* or similar files may first set $DEBUGINFOD_URLS.
> +# If $DEBUGINFOD_URLS is not set there, we set it from system *.url files.
> +# $HOME/.*rc or similar files may then amend $DEBUGINFOD_URLS.
> +# See also [man debuginfod-client-config] for other environment variables
> +# such as $DEBUGINFOD_MAXSIZE, $DEBUGINFOD_MAXTIME, $DEBUGINFOD_PROGRESS.
> +
> +if (! $?DEBUGINFOD_URLS) then
> +set prefix="@prefix@"
> +set debuginfod_urls=`find "@sysconfdir@/debuginfod/" -name '*.urls' |
> xargs cat | tr '\n' ' '`
Can we use cat "@sysconfdir@/debuginfod/*.urls" | tr '\n' ' ' instead
so we don't need to rely on findutils and xargs?
> +if ( "$debuginfod_urls" != "" ) then
> +setenv DEBUGINFOD_URLS "$debuginfod_urls"
> +endif
> +unset debuginfod_urls
> +unset prefix
> endif
> diff --git a/config/profile.sh.in b/config/profile.sh.in
> index aa228a0dcd16..94b2983b9f90 100644
> --- a/config/profile.sh.in
> +++ b/config/profile.sh.in
> @@ -1,4 +1,17 @@
> -if [ -n "@DEBUGINFOD_URLS@" ]; then
> - DEBUGINFOD_URLS="${DEBUGINFOD_URLS-}${DEBUGINFOD_URLS:+
> }@DEBUGINFOD_URLS@"
> - export DEBUGINFOD_URLS
> +
> +# $HOME/.profile* or similar files may first set $DEBUGINFOD_URLS.
> +# If $DEBUGINFOD_URLS is not set there, we set it from system *.url files.
> +# $HOME/.*rc or similar files may then amend $DEBUGINFOD_URLS.
> +# See also [man debuginfod-client-config] for other environment variables
> +# such as $DEBUGINFOD_MAXSIZE, $DEBUGINFOD_MAXTIME, $DEBUGINFOD_PROGRESS.
> +
> +if [ -z "$DEBUGINFOD_URLS" ]; then
> +prefix="@prefix@"
> +debuginfod_urls=`find "@sysconfdir
Re: [PATCH] Tests: Fix warning in show-die-info.c
Hi,
On Tue, Oct 05, 2021 at 05:32:16PM +0200, Jan-Benedict Glaw wrote:
> I'm running automated test compiles on Binutils, GCC, Linux, NetBSD
> and, since a few days ago, elfutils.
>
> Building/running the tests, I noticed this little warning:
>
> [make 2021-10-01 12:18:15] elflint.c: In function 'check_sections':
> [make 2021-10-01 12:18:15] elflint.c:4105:48: error: null pointer dereference
> [-Werror=null-dereference]
> [make 2021-10-01 12:18:15] 4105 | idx <
> databits->d_size && ! bad;
> [make 2021-10-01 12:18:15] |
> ^~~~
> [make 2021-10-01 12:18:18] cc1: all warnings being treated as errors
> [make 2021-10-01 12:18:18] make[2]: *** [Makefile:799: elflint.o] Error 1
> [make 2021-10-01 12:18:18] make[1]: *** [Makefile:532: all-recursive] Error 1
> [make 2021-10-01 12:18:18] make: *** [Makefile:448: all] Error 2
>
>
> As it is tested beforehand that we should not run into this, this
> patch should fix the warning:
>
>
> diff --git a/src/elflint.c b/src/elflint.c
> index 1ce75684..ef7725ce 100644
> --- a/src/elflint.c
> +++ b/src/elflint.c
> @@ -4102,7 +4102,7 @@ section [%2zu] '%s' has type NOBITS but is read from
> the file in segment of prog
> bad = (databits == NULL
> || databits->d_size != shdr->sh_size);
> for (size_t idx = 0;
> - idx < databits->d_size && ! bad;
> + ! bad && idx < databits->d_size;
>idx++)
> bad = ((char *) databits->d_buf)[idx] != 0;
>
Thanks, that warning and the fix look correct.
I committed the attached fix.
Cheers,
Mark
>From 3d9f12883d0c131bd4ab6045e1f60d3fe6d150ea Mon Sep 17 00:00:00 2001
From: Mark Wielaard
Date: Wed, 6 Oct 2021 23:37:42 +0200
Subject: [PATCH] elflint.c: Don't dereference databits if bad
elflint.c: In function 'check_sections':
elflint.c:4105:48: error: null pointer dereference [-Werror=null-dereference]
4105 | idx < databits->d_size && ! bad;
|^~~~
Fix this by testing for ! bad first.
Reported-by: Jan-Benedict Glaw
Signed-off-by: Mark Wielaard
---
src/ChangeLog | 4
src/elflint.c | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 87b3dd46..316bcb6d 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,7 @@
+2021-10-06 Mark Wielaard
+
+ * elflint.c (check_sections): Don't dereference databits if bad.
+
2021-09-09 Dmitry V. Levin
* findtextrel.c: Include "libeu.h".
diff --git a/src/elflint.c b/src/elflint.c
index 1ce75684..ef7725ce 100644
--- a/src/elflint.c
+++ b/src/elflint.c
@@ -4102,7 +4102,7 @@ section [%2zu] '%s' has type NOBITS but is read from the file in segment of prog
bad = (databits == NULL
|| databits->d_size != shdr->sh_size);
for (size_t idx = 0;
- idx < databits->d_size && ! bad;
+ ! bad && idx < databits->d_size;
idx++)
bad = ((char *) databits->d_buf)[idx] != 0;
--
2.32.0
Re: [PATCH] Tests: Fix warning in show-die-info.c
Hi Jan-Benedict,
On Tue, Oct 05, 2021 at 05:36:40PM +0200, Jan-Benedict Glaw wrote:
>
> My last email had a wrong subject, though the patch was correct.
> Here's a second patch, this time *actally* for tests/show-die-info.c:
>
> diff --git a/tests/show-die-info.c b/tests/show-die-info.c
> index 34e27a3b..0823cc60 100644
> --- a/tests/show-die-info.c
> +++ b/tests/show-die-info.c
> @@ -97,7 +97,7 @@ handle (Dwarf *dbg, Dwarf_Die *die, int n)
>printf ("%*s Attrs :", n * 5, "");
>for (cnt = 0; cnt < 0x; ++cnt)
> if (dwarf_hasattr (die, cnt))
> - printf (" %s", dwarf_attr_string (cnt));
> + printf (" %s", (dwarf_attr_string (cnt)? dwarf_attr_string (cnt): ""));
>puts ("");
>
>if (dwarf_hasattr (die, DW_AT_low_pc) && dwarf_lowpc (die, &addr) == 0)
This can be fixed in a shorter way using dwarf_attr_string ?: "".
Which is what I pushed (see attached).
Thanks,
Mark
>From 47b0ebe9033daa7ac9c732b25c85520b97f9635a Mon Sep 17 00:00:00 2001
From: Mark Wielaard
Date: Wed, 6 Oct 2021 23:53:34 +0200
Subject: [PATCH] tests: Handle dwarf_attr_string returning NULL in
show-die-info.c
Reported-by: Jan-Benedict Glaw
Signed-off-by: Mark Wielaard
---
tests/ChangeLog | 4
tests/show-die-info.c | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/tests/ChangeLog b/tests/ChangeLog
index d289b27c..07e018b0 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,7 @@
+2021-10-06 Mark Wielaard
+
+ * show-die-info.c (handle): Handle dwarf_attr_string returning NULL.
+
2021-10-06 Di Chen
PR28242
diff --git a/tests/show-die-info.c b/tests/show-die-info.c
index 34e27a3b..1a3191cd 100644
--- a/tests/show-die-info.c
+++ b/tests/show-die-info.c
@@ -97,7 +97,7 @@ handle (Dwarf *dbg, Dwarf_Die *die, int n)
printf ("%*s Attrs :", n * 5, "");
for (cnt = 0; cnt < 0x; ++cnt)
if (dwarf_hasattr (die, cnt))
- printf (" %s", dwarf_attr_string (cnt));
+ printf (" %s", dwarf_attr_string (cnt) ?: "");
puts ("");
if (dwarf_hasattr (die, DW_AT_low_pc) && dwarf_lowpc (die, &addr) == 0)
--
2.32.0
Re: [patch] PR27783: default debuginfod-urls profile rework
Hi - > > Signed-off-by: Frank Ch. Eigler > > Could you add something about this to NEWS so packagers know how to > update to the new scheme? Sure. > > +set debuginfod_urls=`find "@sysconfdir@/debuginfod/" -name '*.urls' | > > xargs cat | tr '\n' ' '` > > Can we use cat "@sysconfdir@/debuginfod/*.urls" | tr '\n' ' ' instead > so we don't need to rely on findutils and xargs? One problem with that is that several shells (csh, zsh) throw errors when a glob expression has no matches. - FChE
