Re: set sysroot command on AIX has no effect.

["Sangamesh Mallayya" <sangamesh dot swamy at in dot ibm dot com>]

Hi Ulrich,

> 
> > Calling solib_find returning a prefixed sysroot path and the 
duplication
> > can be avoided as you suggested.
> > But calling solib_bfd_fopen after this is causing the assertion to 
fail on
> > NULL path as solib_bfd_fopen doing xfree of pathname at the end.
> 
> Well, of course, you have to check for NULL.  What I'm suggesting is to
> use something along the lines of:
> 
>   found_pathname = solib_find (filename, &found_file);
>   if (found_pathname == NULL)
>     // error handling
>   archive_bfd = solib_bfd_fopen (found_pathname, found_file);
> 

Yes, i did try these steps. But this won't set the sysroot path as we 
intend to.
The final object filename we want is the one returned from solib_find, 
which is a sysroot prefixed pathname.
After solib_bfd_fopen call we can't refer to found_pathname as it's been 
freed in solib_bfd_fopen at the end, and assertion failure later.

> where the code currently does:
> 
>   archive_bfd = gdb_bfd_open (filename, gnutarget, -1);
>   if (archive_bfd == NULL)
>     // error handling
>
> 
> > +   pathname = solib_find (filename, &found_file);
> > +   if (pathname == NULL)
> > +       perror_with_name (filename);
> >     archive_bfd = gdb_bfd_open (filename, gnutarget, -1);
> >     if (archive_bfd == NULL)
> >       {
> 
> This has a number of problems:
> - you still use gdb_bfd_open with filename, which means it still won't
>   find the file  (I assume you meant to use pathname?)

pathname we get is something like "/usr/lib/libc.a(shr.o)", Offcourse 
their is no such file with this pathname in the system.
So we set a filename as "/usr/lib/libc.a" after separating member name 
from actual file and gdb_bfd_open does find the file and return it's bfd.
Here if we pass the path returned from solib_find to gdb_bfd_open instead 
of filename then no issue is seen.

filename is a malloc'ed string which will be freed up when do_cleanups is 
called before existing from solib_aix_bfd_open.


> - if solib_find actually finds the file, "found_file" is an open file
>   descriptor, which the code now leaks

I too was bit reluctant about found_file use after solib_find, but wanted 
to have your view.

> - actually, the pathname string now also leaks
> - and finally, at the bottom:
>   object_bfd->filename = xstrdup (pathname);
>   you now miss the object file name (in parentheses), so
>   "info sharedlibrary" will no longer show it
> 
> Most of those should be fixed when using the approach above.

pathname string we need to assign to object filename is the one which is 
returned from solib_find which gives us sysroot path.
We aren't modifying actual pathname which we got from solib_map_sections, 
just reusing the pathname variable locally, so as to assign that to object 
filename.
I confirmed this with debugging gdb and pasted the output of gdb session 
below.

I think, better we use found_pathname variable to store returned value 
from solib_find instead of using existing pathname variable to avoid 
confusion.
And may assign object filename as "object_bfd->filename = xstrdup 
(found_pathname);" ?

The only problem i think if we use solib_find without using 
solib_bfd_fopen is found_file descriptor.
Let me know your view.

output of gdb session before call to solib_aix_bfd_open & after return 
from solib_aix_bfd_open
----------------------------------------------------------------------------------------------

Breakpoint 3, _ZL18solib_map_sectionsP7so_list (so=0x1102d8bd0) at 
solib.c:547
547       abfd = ops->bfd_open (filename);
(top-gdb) p *so
$7 = {next = 0x1102d9030, lm_info = 0x1102ca310, 
  so_original_name = "/usr/lib/libc.a(shr.o)", '\000' <repeats 489 times>, 

  so_name = "/usr/lib/libc.a(shr.o)", '\000' <repeats 489 times>, pspace = 
0x11025be70, 
  abfd = 0x0, symbols_loaded = 0 '�', objfile = 0x0, sections = 0x0, 
sections_end = 0x0, 
  addr_low = 0, addr_high = 0}
(top-gdb) p filename
$8 = 0x110273b70 "/usr/lib/libc.a(shr.o)"
(top-gdb) n
548       do_cleanups (old_chain);
(top-gdb) p *so
$9 = {next = 0x1102d9030, lm_info = 0x1102ca310, 
  so_original_name = "/usr/lib/libc.a(shr.o)", '\000' <repeats 489 times>, 

  so_name = "/usr/lib/libc.a(shr.o)", '\000' <repeats 489 times>, pspace = 
0x11025be70, 
  abfd = 0x0, symbols_loaded = 0 '�', objfile = 0x0, sections = 0x0, 
sections_end = 0x0, 
  addr_low = 0, addr_high = 0}
(top-gdb) p filename
$10 = 0x110273b70 "/usr/lib/libc.a(shr.o)"
(top-gdb) 

Thanks,
-Sangamesh




From:   "Ulrich Weigand" <uweigand@de.ibm.com>
To:     Sangamesh Mallayya/India/IBM@IBMIN
Cc:     gdb-patches@sourceware.org
Date:   10/08/2016 01:01 AM
Subject:        Re: set sysroot command on AIX has no effect.



Sangamesh Mallaya wrote:

> Calling solib_find returning a prefixed sysroot path and the duplication
> can be avoided as you suggested.
> But calling solib_bfd_fopen after this is causing the assertion to fail 
on
> NULL path as solib_bfd_fopen doing xfree of pathname at the end.

Well, of course, you have to check for NULL.  What I'm suggesting is to
use something along the lines of:

  found_pathname = solib_find (filename, &found_file);
  if (found_pathname == NULL)
    // error handling
  archive_bfd = solib_bfd_fopen (found_pathname, found_file);

where the code currently does:

  archive_bfd = gdb_bfd_open (filename, gnutarget, -1);
  if (archive_bfd == NULL)
    // error handling


> +   pathname = solib_find (filename, &found_file);
> +   if (pathname == NULL)
> +       perror_with_name (filename);
>     archive_bfd = gdb_bfd_open (filename, gnutarget, -1);
>     if (archive_bfd == NULL)
>       {

This has a number of problems:
- you still use gdb_bfd_open with filename, which means it still won't
  find the file  (I assume you meant to use pathname?)
- if solib_find actually finds the file, "found_file" is an open file
  descriptor, which the code now leaks
- actually, the pathname string now also leaks
- and finally, at the bottom:
  object_bfd->filename = xstrdup (pathname);
  you now miss the object file name (in parentheses), so
  "info sharedlibrary" will no longer show it

Most of those should be fixed when using the approach above.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com