Re: [PATCH 0/2] elfutils: minor build system fixes
On Wed, 2019-12-11 at 16:23 -0800, Omar Sandoval wrote: > This small series fixes a couple of issues I encountered when building > elfutils. Patch 1 fixes an issue when compiling with CFLAGS='-Wno-error > -O0'. Patch 2 gets rid of a warning. Both look good, pushed to master. Thanks. Do you still need -Wno-error? Cheers, Mark
Re: [PATCH 0/2] elfutils: minor build system fixes
On Thu, Dec 12, 2019 at 12:17:23PM +0100, Mark Wielaard wrote: > On Wed, 2019-12-11 at 16:23 -0800, Omar Sandoval wrote: > > This small series fixes a couple of issues I encountered when building > > elfutils. Patch 1 fixes an issue when compiling with CFLAGS='-Wno-error > > -O0'. Patch 2 gets rid of a warning. > > Both look good, pushed to master. Thanks. > > Do you still need -Wno-error? Nope, at least with GCC 9.2.0 that I'm using, it builds cleanly. Thanks!
Re: rfc/patch: debuginfod client $DEBUGINFOD_PROGRESS env var
Hi - > > debuginfod: usability tweaks, incl. $DEBUGINFOD_PROGRESS client > > support > > > > This facility allows a default progress-printing function > > to be installed if the given environment variable is set. > > I like this idea but have a bit of a security concern about the ability > to set it to any file. If someone manages to set it then they can > overwrite anything a process using the library can write to. That's not that serious a category of concern. Environment variables are not under control of untrusted agents. FWIW, $DEBUGINFOD_CACHE is considerably more dangerous in that regard (cache cleaning!). > I rather it would just use stderr always when set since it is only > meant as a human debugging device. That default makes sense. Making it configurable gives a way for a larger app to still capture output (by having the client direct it to a pipe file descriptor or something), but I'll just hardcode for now STDERR_FILENO if the env var is set. > > Some larger usage experience (systemtap/kernels) indicates > > the default timeout is too short, so bumped it to 30s. > My first thought is that we might need two timeouts. The original 5s > timeout is a good inactivity timeout [...] But for larger data we > might want a timeout for the total download time, maybe 30 seconds > is too low for that. Given how large kernel debuginfo is it might > take several minutes. OK, redrafting $DEBUGINFOD_TIMEOUT as a two-part variable: $DEBUGINFOD_TIMEOUT=x,y x - connection timeout, default 5 y - optional, overall timeout, default unlimited > [...] > This seems to mean that if there is an error then the progress function > is never called, is that intended? Also for CURLM_CALL_MULTI_PERFORM we > continue/skip the progress function. If we have an error outcome, there is no more progress to report, so that should be OK. > > + close (fd); /* before the rmdir, otherwise it'll fail */ > >(void) rmdir (target_cache_dir); /* nop if not empty */ > >free(data); > > - close (fd); > > This looks good. Just surprising. I assumed unlinking the file was > enough. Yeah, I had to see that for myself to believe it. > The \r \n trick is nice. > Just always using stderr would simplify this a bit. OK. > Also note that you can use dprintf to printf to a file descriptor. OK. - FChE
Re: [PATCH 0/4] libdwfl: make dwfl_addrmodule work for Linux kernel modules
On Wed, Dec 11, 2019 at 05:29:42PM -0800, Omar Sandoval wrote:
> From: Omar Sandoval
>
> Hello,
>
> I recently encountered a bug that dwfl_addrmodule doesn't work correctly
> for Linux kernel modules. This is because each section of a kernel
> module is allocated independently, so sections from different kernel
> modules may be intermixed. For example:
>
> # cd /sys/modules
> # cat ext4/sections/.init.text
> 0xc0f0f000
> # cat ext4/sections/.bss
> 0xc1303e80
> # cat kvm/sections/.init.text
> 0xc0f06000
> # cat kvm/sections/.bss
> 0xc10d2340
>
> This confuses dwfl_addrmodule/dwfl_addrsegment, which builds a lookup
> table based on mod->low_addr and mod->high_addr.
I did some more testing, and I realized that my analysis was wrong :(
What's going on here is that:
1. The kernel frees the .init sections once the module is initialized,
which means the addresses can be reused for new modules.
2. My application is reporting low_addr and high_addr based on the
section addresses (which is different from how
dwfl_linux_kernel_report_modules does it).
Reading the kernel code, the main sections are indeed contiguous. So
this was entirely my bug. Sorry for the noise!
On the bright side, patch 2 ("libdwfl: remove broken coalescing logic in
dwfl_report_segment") does seem like a legitimate bug.
