Some debuginfod fixlets

2019-11-16 Thread Mark Wielaard
Hi,

While testing Frank's new spec/rpms for the run-debuginfod-find.sh
testcase I found a couple of issues that I pushed to the debuginfod-
submit branch.

  Add tests/debuginfod-rpms to EXTRA_DISTS.
  Fix two small memory leaks in debuginfod-find and testcase.
  Increase timeout for run-debuginfod-find.sh
  run-debuginfod-find.sh: Use abs_srcdir when copying debuginfod-rpms
  debuginfod: Accept empty comp_dir when cuname is absolute.

With these fixes everything passes make distcheck locally.

The only interesting one is debuginfod: Accept empty comp_dir when
cuname is absolute. This can happen with gcc 4.8.5. In that case we
have a CU DIE with an absolute DW_AT_name, but no DW_AT_comp_dir. And
the files in the debug_line table never reference the comp_dir (dir
entry zero). For example:

 [ b]  compile_unit abbrev: 1
   producer (strp) "GNU C 4.8.5 20150623 (Red Hat 4.8.5-39) 
-mtune=generic -march=x86-64 -g"
   language (data1) C89 (1)
   name (strp) "/home/mark/build/elfutils-obj/prog.c"
   low_pc   (addr) 0x004004ed
   high_pc  (data8) 11 (0x004004f8)
   stmt_list(sec_offset) 0

Directory table:
 /home/mark/build/elfutils-obj

File name table:
 Entry Dir   Time  Size  Name
 1 1 0 0 prog.c

Hopefully these changes are correct. If not, please let me know, and
apologies for pushing them on your branch.

Cheers,

Mark


Re: patch 1/2 debuginfod client

2019-11-16 Thread Frank Ch. Eigler
Hi -

> It also seems that how you did it on the branch:

Yeah, maybe a different automake version made it work after my earlier
failing tests.  (dead horse PS.  It remains my opinion that we should
commit auto* generated files into git.)

> > Possible, but since these bits are just getting transcribed straight
> > to a debuginfod URL, an insane unclean hex will just get rejected at
> > that time.  There is no lost-error case there.
> 
> But since the user won't see the URL generated they might not notice
> what is really going on. They will just see that something wasn't
> found, won't they?

Yes, so the only benefit would be the generation of a different error
message for impossible buildids.


> > Yeah ... if a user were to set DEBUGINFOD_CACHE_DIR to $HOME, she
> > will
> > get a good thorough cleaning, or a root user were to set it to /.
> > PEBCAK IMHO; don't see easy ways of protecting themselves against it.
> 
> It might be unlikely the user accidentally set the environment
> variables to something odd, 

> but they might be tricked into running it on a debug file that was
> doctored. Which might produce lookups for odd source files. It might
> even just be a buggy compiler that created a few ../.. too many. It
> would be bad if that would cause havoc.

Source file names do not survive into the client side cache - you 
already found the "#" escaping bits.


> > > > +/* NB: these are thread-unsafe. */
> > > > +__attribute__((constructor)) attribute_hidden void 
> > > > libdebuginfod_ctor(void)
> > > > +{
> > > > +  curl_global_init(CURL_GLOBAL_DEFAULT);
> > > > +}
> > > 
> > > How does this interact with a program that uses libcurl itself and also
> > > links with libdebuginfod?
> > 
> > The call is harmless if repeated.  It is described merely as
> > multi-thread-unsafe.
> 
> It also says:
> 
>This function is not thread safe.
>You must not call it when any other thread in the program (i.e. a  thread 
> sharing the same memory) is running. This doesn't just mean no other thread 
> that is using libcurl.  Because curl_global_init calls functions of other 
> libraries that are similarly thread unsafe,
>it could conflict with any other thread that uses these other libraries.
> 
> That doesn't make me very happy.
> It looks like using libcurl from another library is not really very
> safe if the program or another library it links against are also
> libcurl users.
> 
> Do you know how other libraries that use libcurl deal with this?

I looked over some other libcurl users in fedora.  Some don't worry
about the issue at all, relying on implicit initialization, which is
only safe if single-threaded.  Others (libvirtd) do an explicitly
invoked initialization function, which is also only safe if invoked
from a single-threaded context.

I think actually our way here, running the init function from the
shared library constructor, is about the best possible.  As long as
the ld.so process is done as normal, it should be fine.  Programs that
use the elfutils trick of manual dlopen'ing libdebuginfod.so should do
so only if they are single-threaded.


> > These are the webapi URL strings.  The cache file names are not the
> > same - those are specifically scrubbed with the '#' characters.
> 
> Aha. That is done by this code:
> 
> >   /* copy the filename to suffix, s,/,#,g */
> >   for (q=0; q > {
> >   if (filename[q] == '\0') break;
> >   if (filename[q] == '/' || filename[q] == '.') suffix[q] = '#';
> >   else suffix[q] = filename[q];
> > }
> 
> Why do you also replace '.' with '#'?
> That seems unnecessary.

It was to make sure ../ names from dwarf source files definitely don't
escape the cache dir.  But yeah if we nuke "/", then "." is alone
safe, and actually that would be good.


> What about files that already contain '#' chars?
> Wouldn't something like /foo/bar#/baz clash with /foo/bar/#baz?
> Or do you just think that is just completely unlikely to ever occur?

Considered it unlikely, but will tweak the code to escape these two
without clashing.


> > > I assume libcurl handles tls certificates for https? Does that need any
> > > mention here?
> > 
> > Dunno if it's worth discussing ... the client side of https usually
> > does not deal with configurable certificates.
> 
> But the client side might or might not verify the server side ssl
> certificate. Does it do that? And if so, which trusted roots does it
> use? And can that be disabled or changed?

Whatever the compiled-in defaults are, /etc paths etc.  IMHO we
shouldn't worry about them.


> > I suppose that wouldn't make any sense.  Anything will take a nonzero
> > amount of time. :-) Maybe it could be a floating point number or
> > something; worth it?
> 
> I was more thinking zero == infinity (no timeout).

An unset environment variable should do that.

- FChE



[Bug libdw/25173] dwarf_getsrc_die fails for rust code

2019-11-16 Thread mail at milianw dot de
https://sourceware.org/bugzilla/show_bug.cgi?id=25173

--- Comment #2 from Milian Wolff  ---
Created attachment 12080
  --> https://sourceware.org/bugzilla/attachment.cgi?id=12080&action=edit
binary for reproduction

here's the compressed binary as compiled with rustc 1.39.0 (4560ea788
2019-11-04)

note that this one is compiled today, so the output is a bit different:

```
$ addr2line -f -i -C -e ./main -a 227f0
0x000227f0
core::fmt::Arguments::new_v1
/rustc/4560ea788cb760f0a34127156c78e2552949f734//src/libcore/fmt/mod.rs:316
core::str::slice_error_fail
/rustc/4560ea788cb760f0a34127156c78e2552949f734//src/libcore/str/mod.rs:2068
```

-- 
You are receiving this mail because:
You are on the CC list for the bug.