Re: set sysroot command on AIX has no effect.

Re: set sysroot command on AIX has no effect.

[Sangamesh Mallayya]

Hi,

Please review the attached patch and let me know the comments.

Thanks & Regards,
-Sangamesh
AIX BOS Development



From:   Sangamesh Mallayya/India/IBM
To:     gdb-patches@sourceware.org
Date:   09/13/2016 02:34 PM
Subject:        set sysroot command on AIX has no effect.


set sysroot command on AIX has no effect if a program depends on shared 
library archives (.a).

This is the original behaviour if we use "set sysroot" command.

(gdb) set sysroot /tmp
warning: while parsing aix library list: no element found
(gdb) file /home/sysroot/testApp
Reading symbols from /home/sysroot/testApp...done.
(gdb) core-file /home/sysroot/core
warning: core file may not match specified executable file.
warning: Unexpected size of section `.reg' in core file.
warning: Could not open `/home/sysroot/libTest.a' as an executable file: A 
file or directory in the path name does not exist.
warning: Could not load shared library symbols for 
/home/sysroot/libTest.a(func.o).
Do you need "set solib-search-path" or "set sysroot"?
Program terminated with signal SIGABRT, Aborted.
warning: Unexpected size of section `.reg' in core file.
#0  0xd013a820 in raise () from /usr/lib/libc.a(shr.o)
(gdb) info sharedlibrary 
From        To          Syms Read   Shared Object Library
0xd04da240  0xd04daa3e  Yes (*)     /usr/lib/libcrypt.a(shr.o)
                                        No /home/sysroot/libTest.a(func.o)
0xd010c880  0xd049b13d  Yes (*)     /usr/lib/libc.a(shr.o)
(*): Shared library is missing debugging information.
(gdb) bt
#0  0xd013a820 in raise () from /usr/lib/libc.a(shr.o)
#1  0xd01b3088 in abort () from /usr/lib/libc.a(shr.o)
#2  0xd2fd82a4 in ?? ()
#3  0x100003a4 in main () at main.c:8
(gdb) 
 

After applying the attached patch.
Patch doesn't cause any change in original behaviour while handling shared 
library with .so names.

".a" name archives case.

(gdb) set sysroot /tmp 
warning: while parsing aix library list: no element found
(gdb) file /home/sysroot/testApp 
Reading symbols from /home/sysroot/testApp...done.
(gdb) core-file /home/sysroot/core
warning: core file may not match specified executable file.
warning: Unexpected size of section `.reg' in core file.
Program terminated with signal SIGABRT, Aborted.
warning: Unexpected size of section `.reg' in core file.
#0  0xd013a820 in raise () from /tmp/usr/lib/libc.a(shr.o)
(gdb) inf sharedlibrary 
From             To                   Syms Read   Shared Object Library
0xd04da240  0xd04daa3e  Yes    (*)       /tmp/usr/lib/libcrypt.a(shr.o)
0xd2fd6250   0xd2fd64aa   Yes /tmp/home/sysroot/libTest.a(func.o)
0xd010c880  0xd049b13d  Yes    (*)       /tmp/usr/lib/libc.a(shr.o)
(*): Shared library is missing debugging information.
(gdb) bt
#0  0xd013a820 in raise () from /tmp/usr/lib/libc.a(shr.o)
#1  0xd01b3088 in abort () from /tmp/usr/lib/libc.a(shr.o)
#2  0xd2fd6364 in func () from /tmp/home/sysroot/libTest.a(func.o)
#3  0x100003a4 in main () at main.c:8
(gdb) 


.a & .so library names

(gdb) set sysroot /tmp
warning: while parsing aix library list: no element found
(gdb) file /home/sysroot/testApp 
Reading symbols from /home/sysroot/testApp...done.
(gdb) core-file /home/sysroot/core
warning: core file may not match specified executable file.
warning: Unexpected size of section `.reg' in core file.
Program terminated with signal SIGABRT, Aborted.
warning: Unexpected size of section `.reg' in core file.
#0  0xd013a820 in raise () from /tmp/usr/lib/libc.a(shr.o)
(gdb) info sharedlibrary 
From             To                   Syms Read   Shared Object Library
0xd04da240  0xd04daa3e  Yes    (*)        /tmp/usr/lib/libcrypt.a(shr.o)
0xd2536250  0xd25499fc   Yes 
/tmp/opt/freeware/lib/gcc/powerpc-ibm-aix6.1.0.0/4.8.5/libgcc_s.a(shr.o)
0xd0566c80  0xd0566db6  Yes    (*)        /tmp/usr/lib/librtl.a(shr.o)
0xd010c880  0xd049b13d  Yes    (*)        /tmp/usr/lib/libc.a(shr.o)
0xd2fd7150   0xd2fd78e1   Yes               /tmp/home/sysroot/libTest.so
(*): Shared library is missing debugging information.
(gdb) bt
#0  0xd013a820 in raise () from /tmp/usr/lib/libc.a(shr.o)
#1  0xd01b3088 in abort () from /tmp/usr/lib/libc.a(shr.o)
#2  0xd2fd749c in func () from /tmp/home/sysroot/libTest.so
#3  0x100003fc in main () at main.c:8
(gdb)


Please review and let me know the comments.

[attachment "ChangeLog" deleted by Sangamesh Mallayya/India/IBM] 
[attachment "set_sysroot.patch" deleted by Sangamesh Mallayya/India/IBM] 

Thanks & Regards,
-Sangamesh
AIX BOS Development

set sysroot command on AIX has no effect.

[Sangamesh Mallayya]

[-- Attachment #1: Type: text/plain, Size: 4153 bytes --]

set sysroot command on AIX has no effect if a program depends on shared 
library archives (.a).

This is the original behaviour if we use "set sysroot" command.

(gdb) set sysroot /tmp
warning: while parsing aix library list: no element found
(gdb) file /home/sysroot/testApp
Reading symbols from /home/sysroot/testApp...done.
(gdb) core-file /home/sysroot/core
warning: core file may not match specified executable file.
warning: Unexpected size of section `.reg' in core file.
warning: Could not open `/home/sysroot/libTest.a' as an executable file: A 
file or directory in the path name does not exist.
warning: Could not load shared library symbols for 
/home/sysroot/libTest.a(func.o).
Do you need "set solib-search-path" or "set sysroot"?
Program terminated with signal SIGABRT, Aborted.
warning: Unexpected size of section `.reg' in core file.
#0  0xd013a820 in raise () from /usr/lib/libc.a(shr.o)
(gdb) info sharedlibrary 
From        To          Syms Read   Shared Object Library
0xd04da240  0xd04daa3e  Yes (*)     /usr/lib/libcrypt.a(shr.o)
                                        No /home/sysroot/libTest.a(func.o)
0xd010c880  0xd049b13d  Yes (*)     /usr/lib/libc.a(shr.o)
(*): Shared library is missing debugging information.
(gdb) bt
#0  0xd013a820 in raise () from /usr/lib/libc.a(shr.o)
#1  0xd01b3088 in abort () from /usr/lib/libc.a(shr.o)
#2  0xd2fd82a4 in ?? ()
#3  0x100003a4 in main () at main.c:8
(gdb) 
 

After applying the attached patch.
Patch doesn't cause any change in original behaviour while handling shared 
library with .so names.

".a" name archives case.

(gdb) set sysroot /tmp 
warning: while parsing aix library list: no element found
(gdb) file /home/sysroot/testApp 
Reading symbols from /home/sysroot/testApp...done.
(gdb) core-file /home/sysroot/core
warning: core file may not match specified executable file.
warning: Unexpected size of section `.reg' in core file.
Program terminated with signal SIGABRT, Aborted.
warning: Unexpected size of section `.reg' in core file.
#0  0xd013a820 in raise () from /tmp/usr/lib/libc.a(shr.o)
(gdb) inf sharedlibrary 
From             To                   Syms Read   Shared Object Library
0xd04da240  0xd04daa3e  Yes    (*)       /tmp/usr/lib/libcrypt.a(shr.o)
0xd2fd6250   0xd2fd64aa   Yes /tmp/home/sysroot/libTest.a(func.o)
0xd010c880  0xd049b13d  Yes    (*)       /tmp/usr/lib/libc.a(shr.o)
(*): Shared library is missing debugging information.
(gdb) bt
#0  0xd013a820 in raise () from /tmp/usr/lib/libc.a(shr.o)
#1  0xd01b3088 in abort () from /tmp/usr/lib/libc.a(shr.o)
#2  0xd2fd6364 in func () from /tmp/home/sysroot/libTest.a(func.o)
#3  0x100003a4 in main () at main.c:8
(gdb) 


.a & .so library names

(gdb) set sysroot /tmp
warning: while parsing aix library list: no element found
(gdb) file /home/sysroot/testApp 
Reading symbols from /home/sysroot/testApp...done.
(gdb) core-file /home/sysroot/core
warning: core file may not match specified executable file.
warning: Unexpected size of section `.reg' in core file.
Program terminated with signal SIGABRT, Aborted.
warning: Unexpected size of section `.reg' in core file.
#0  0xd013a820 in raise () from /tmp/usr/lib/libc.a(shr.o)
(gdb) info sharedlibrary 
From             To                   Syms Read   Shared Object Library
0xd04da240  0xd04daa3e  Yes    (*)        /tmp/usr/lib/libcrypt.a(shr.o)
0xd2536250  0xd25499fc   Yes 
/tmp/opt/freeware/lib/gcc/powerpc-ibm-aix6.1.0.0/4.8.5/libgcc_s.a(shr.o)
0xd0566c80  0xd0566db6  Yes    (*)        /tmp/usr/lib/librtl.a(shr.o)
0xd010c880  0xd049b13d  Yes    (*)        /tmp/usr/lib/libc.a(shr.o)
0xd2fd7150   0xd2fd78e1   Yes               /tmp/home/sysroot/libTest.so
(*): Shared library is missing debugging information.
(gdb) bt
#0  0xd013a820 in raise () from /tmp/usr/lib/libc.a(shr.o)
#1  0xd01b3088 in abort () from /tmp/usr/lib/libc.a(shr.o)
#2  0xd2fd749c in func () from /tmp/home/sysroot/libTest.so
#3  0x100003fc in main () at main.c:8
(gdb)


Please review and let me know the comments.



Thanks & Regards,
-Sangamesh
AIX BOS Development

[-- Attachment #2: ChangeLog --]
[-- Type: application/octet-stream, Size: 120 bytes --]

	*solib-aix.c (solib_aix_bfd_open): Recognise sysroot path if a program on AIX
	 requires shared library archives (.a).

[-- Attachment #3: set_sysroot.patch --]
[-- Type: application/octet-stream, Size: 1197 bytes --]

--- ./gdb/solib-aix.c.sysroot   2016-02-09 21:19:39.000000000 -0600
+++ ./gdb/solib-aix.c   2016-09-13 02:21:22.000000000 -0500
@@ -642,17 +642,28 @@
 
      FIXME: This is a little hacky.  Perhaps we should provide access
      to the solib's lm_info here?  */
-  const int path_len = strlen (pathname);
+  int path_len = strlen (pathname);
   char *sep;
   char *filename;
   int filename_len;
-  char *member_name;
+  char *member_name, *sys_path;
   bfd *archive_bfd, *object_bfd;
   struct cleanup *cleanup;
 
-  if (pathname[path_len - 1] != ')')
+  if (pathname[path_len - 1] != ')') /* .so case */
     return solib_bfd_open (pathname);
 
+  /* If gdb_sysroot is set via set sysroot command, 
+     then we need to prefix gdb_sysroot to shared library path  */
+  /* .a shared library archives case */
+  if (gdb_sysroot != NULL && *gdb_sysroot != 0) {
+      sys_path = alloca (sizeof(gdb_sysroot)+strlen(pathname)); 
+      memset (sys_path, 0, sizeof(sys_path));
+      strcpy(sys_path, gdb_sysroot);
+      pathname = strcat (sys_path, pathname);
+  }
+  path_len = strlen (pathname);
+
   /* Search for the associated parens.  */
   sep = strrchr (pathname, '(');
   if (sep == NULL)

Re: set sysroot command on AIX has no effect.

[Ulrich Weigand]

Sangamesh Mallaya wrote:

> set sysroot command on AIX has no effect if a program depends on shared
> library archives (.a).

Yes, that seems a bug.

> +  /* If gdb_sysroot is set via set sysroot command, 
> +     then we need to prefix gdb_sysroot to shared library path  */
> +  /* .a shared library archives case */
> +  if (gdb_sysroot != NULL && *gdb_sysroot != 0) {
> +      sys_path = alloca (sizeof(gdb_sysroot)+strlen(pathname)); 
> +      memset (sys_path, 0, sizeof(sys_path));
> +      strcpy(sys_path, gdb_sysroot);
> +      pathname = strcat (sys_path, pathname);
> +  }
> +  path_len = strlen (pathname);

This duplicates some of the logic in solib_find.  It would be better
to avoid that duplication ...

Is there any reason why you cannot use solib_find / solib_bfd_fopen
to open the archive file, like solib_bfd_open does?  This would ensure
we're always using the same search logic for .a files as for .so files.

Bye,
Ulrich

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

Re: set sysroot command on AIX has no effect.

[Sangamesh Mallayya]

Hi Ulrich,

Thanks for the review and comments.

If we call solib_bfd_open for .a files then the call to open file using 
gdb_open_cloexec inside solib_find function is failing.
The reason for this is,
we will be getting library information from ld_info in the form of 
"/usr/lib/libc.a(shr.o)", but actually we will be having the
file in filesystem as /usr/lib/libc.a. This is the reason initially i 
didn't use solib_bfd_open.

But i experimented some more and tried to see how gdb behaves if we pass 
modified pathname exactly as the file which we are having in the machine, 
like "/usr/lib/libc.a".
Then bfd bfd_check_format is failing with the error "not in executable 
format".
Looks like bfd isn't able to recognise the .a archives.
I can have a look on why bfd check is failing.
Please let me know if this is the right way to move forward or you have 
any suggestions.


Thanks,
-Sangamesh




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



Sangamesh Mallaya wrote:

> set sysroot command on AIX has no effect if a program depends on shared
> library archives (.a).

Yes, that seems a bug.

> +  /* If gdb_sysroot is set via set sysroot command, 
> +     then we need to prefix gdb_sysroot to shared library path  */
> +  /* .a shared library archives case */
> +  if (gdb_sysroot != NULL && *gdb_sysroot != 0) {
> +      sys_path = alloca (sizeof(gdb_sysroot)+strlen(pathname)); 
> +      memset (sys_path, 0, sizeof(sys_path));
> +      strcpy(sys_path, gdb_sysroot);
> +      pathname = strcat (sys_path, pathname);
> +  }
> +  path_len = strlen (pathname);

This duplicates some of the logic in solib_find.  It would be better
to avoid that duplication ...

Is there any reason why you cannot use solib_find / solib_bfd_fopen
to open the archive file, like solib_bfd_open does?  This would ensure
we're always using the same search logic for .a files as for .so files.

Bye,
Ulrich

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

Re: set sysroot command on AIX has no effect.

[Ulrich Weigand]

Hi Sangamesh,

> If we call solib_bfd_open for .a files then the call to open file using
> gdb_open_cloexec inside solib_find function is failing.
> The reason for this is,
> we will be getting library information from ld_info in the form of
> "/usr/lib/libc.a(shr.o)", but actually we will be having the
> file in filesystem as /usr/lib/libc.a. This is the reason initially i
> didn't use solib_bfd_open.

Yes, I understand why you cannot use solib_bfd_open directly.  That's why
I suggested to use *solib_bfd_fopen* (note the "f") :-)

> But i experimented some more and tried to see how gdb behaves if we pass
> modified pathname exactly as the file which we are having in the machine,
> like "/usr/lib/libc.a".
> Then bfd bfd_check_format is failing with the error "not in executable
> format".
> Looks like bfd isn't able to recognise the .a archives.
> I can have a look on why bfd check is failing.

That's because solib_bfd_open uses the bfd_object argument to
bfd_check_format, which is explicitly supposed to reject .a archives.
Instead, to check for archives, you need to use bfd_archive, like
solib_aix_bfd_open does.  I'm not suggesting to change that, just to
open the bfd using solib_find and solib_bfd_fopen before performing
the check, instead of using gdb_bfd_open.

Bye,
Ulrich

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

Re: set sysroot command on AIX has no effect.

[Sangamesh Mallayya]

Hi Ulrich,


> 
> > If we call solib_bfd_open for .a files then the call to open file 
using
> > gdb_open_cloexec inside solib_find function is failing.
> > The reason for this is,
> > we will be getting library information from ld_info in the form of
> > "/usr/lib/libc.a(shr.o)", but actually we will be having the
> > file in filesystem as /usr/lib/libc.a. This is the reason initially i
> > didn't use solib_bfd_open.
> 
> Yes, I understand why you cannot use solib_bfd_open directly.  That's 
why
> I suggested to use *solib_bfd_fopen* (note the "f") :-)
> 
> > But i experimented some more and tried to see how gdb behaves if we 
pass
> > modified pathname exactly as the file which we are having in the 
machine,
> > like "/usr/lib/libc.a".
> > Then bfd bfd_check_format is failing with the error "not in executable
> > format".
> > Looks like bfd isn't able to recognise the .a archives.
> > I can have a look on why bfd check is failing.
> 
> That's because solib_bfd_open uses the bfd_object argument to
> bfd_check_format, which is explicitly supposed to reject .a archives.
> Instead, to check for archives, you need to use bfd_archive, like
> solib_aix_bfd_open does.  I'm not suggesting to change that, just to
> open the bfd using solib_find and solib_bfd_fopen before performing
> the check, instead of using gdb_bfd_open.

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.
As we still want to refer to sysroot path returned from solib_find or 
filename inside solib_aix_bfd_open until assigning path to bfd filename "(
object_bfd->filename = xstrdup (pathname)"
So calling solib_bfd_fopen is causing assertion failure like this.

utils.c:2971: internal-error: char* gdb_abspath(const char*): Assertion 
`path != NULL && path[0] != '\0'' failed. 

This is the change i have it right now.

    char *member_name;
    bfd *archive_bfd, *object_bfd;
    struct cleanup *cleanup;
+   int found_file;
 
    if (pathname[path_len - 1] != ')')
      return solib_bfd_open (pathname);
@@ -669,6 +670,9 @@
    member_name = xstrprintf ("%.*s", path_len - filename_len - 2, sep + 
1);
    make_cleanup (xfree, member_name);
 
+   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)
      {

Let me know your view.

Re: set sysroot command on AIX has no effect.

[Ulrich Weigand]

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

Re: set sysroot command on AIX has no effect.

[Sangamesh Mallayya]

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

Re: set sysroot command on AIX has no effect.

[Ulrich Weigand]

Sangamesh Mallayya wrote:

>> 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.

Well, if you want to reuse the found_pathname, you'll just have to
duplicate it (there's already a call to xstrdup later in the function,
you may have to move it earlier).

>> > +   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.

My point was, the code as shown above does *not* pass the path returned
from solib_find to gdb_bfd_open!  The path returned from solib_find is
assigned to "pathname" (overriding the value pathname that was input to
the function), but then gdb_bfd_open is still called with "filename".

>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);" ?

Agreed (see above).  However, note that this will *not* contain the
object name in parentheses.  For example, if you initially get a
pathname of:
  /usr/lib/libc.a(shr.o)
Then you separate out the filename into:
  /usr/lib/libc.a
Then you pass this to solib_find and get a full name including sysroot:
  /path/to/sysroot/usr/lib/libc.a
But what you probably want to use as name in the final solib object is:
  /path/to/sysroot/usr/lib/libc.a(shr.o)

My point was that in order to get this, you'll have to append the
"(shr.o)" back on to the result you got from solib_find.

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

Please do use the pair of solib_find / solib_bfd_fopen; they were
designed to be used with each other, that's why they have the
interface they do.

Bye,
Ulrich

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

Re: set sysroot command on AIX has no effect.

[Sangamesh Mallayya]

Hi Ulrich,

> Sangamesh Mallayya wrote:
> 
> >> 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.
> 
> Well, if you want to reuse the found_pathname, you'll just have to
> duplicate it (there's already a call to xstrdup later in the function,
> you may have to move it earlier).
> 
> >> > +   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.
> 
> My point was, the code as shown above does *not* pass the path returned
> from solib_find to gdb_bfd_open!  The path returned from solib_find is
> assigned to "pathname" (overriding the value pathname that was input to
> the function), but then gdb_bfd_open is still called with "filename".
> 
> >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);" ?
> 
> Agreed (see above).  However, note that this will *not* contain the
> object name in parentheses.  For example, if you initially get a
> pathname of:
>   /usr/lib/libc.a(shr.o)
> Then you separate out the filename into:
>   /usr/lib/libc.a
> Then you pass this to solib_find and get a full name including sysroot:
>   /path/to/sysroot/usr/lib/libc.a
> But what you probably want to use as name in the final solib object is:
>   /path/to/sysroot/usr/lib/libc.a(shr.o)
> 
> My point was that in order to get this, you'll have to append the
> "(shr.o)" back on to the result you got from solib_find.
> 
> >The only problem i think if we use solib_find without using 
> >solib_bfd_fopen is found_file descriptor.
> >Let me know your view.
> 
> Please do use the pair of solib_find / solib_bfd_fopen; they were
> designed to be used with each other, that's why they have the
> interface they do.

Thanks a lot for the detailed explanation and guidance.

Here is the change i have it right now which takes care of.

1) calling solib_find and then solib_bfd_fopen.
2) Appending parenthesized member name.

Let me know your view on this.

--- ./gdb/solib-aix.c_orig      2016-10-04 03:22:01.000000000 -0500
+++ ./gdb/solib-aix.c   2016-10-10 06:23:32.000000000 -0500
@@ -648,6 +648,8 @@
    char *member_name;
    bfd *archive_bfd, *object_bfd;
    struct cleanup *cleanup;
+   int found_file, found_path_len;
+   char *found_pathname, *found_pathname1;
 
    if (pathname[path_len - 1] != ')')
      return solib_bfd_open (pathname);
@@ -669,12 +671,24 @@
    member_name = xstrprintf ("%.*s", path_len - filename_len - 2, sep + 
1);
    make_cleanup (xfree, member_name);
 
-   archive_bfd = gdb_bfd_open (filename, gnutarget, -1);
+   /* Calling solib_find makes certain that sysroot path is set properly
+      if program has a dependency on .a archive and sysroot is set via
+      set sysroot command */
+   found_pathname = solib_find (filename, &found_file);
+   if (found_pathname == NULL)
+       perror_with_name (pathname);
+   found_pathname1 = xstrdup (found_pathname);
+   found_path_len = strlen (found_pathname1) + strlen (sep);
+   /* Reserve a space for appending parenthesized member name to 
filename,
+      as path returned from solib_find is just a filename */
+   found_pathname1 = xrealloc (found_pathname1, found_path_len);
+   archive_bfd = solib_bfd_fopen (found_pathname, found_file);
    if (archive_bfd == NULL)
      {
        warning (_("Could not open `%s' as an executable file: %s"),
               filename, bfd_errmsg (bfd_get_error ()));
        do_cleanups (cleanup);
+       xfree (found_pathname1);
        return NULL;
      }
 
@@ -681,6 +695,7 @@
    if (bfd_check_format (archive_bfd, bfd_object))
      {
        do_cleanups (cleanup);
+       xfree (found_pathname1);
        return archive_bfd;
      }
 
@@ -690,6 +705,7 @@
               filename, bfd_errmsg (bfd_get_error ()));
        gdb_bfd_unref (archive_bfd);
        do_cleanups (cleanup);
+       xfree (found_pathname1);
        return NULL;
      }
 
@@ -711,6 +727,7 @@
        warning (_("\"%s\": member \"%s\" missing."), filename, 
member_name);
        gdb_bfd_unref (archive_bfd);
        do_cleanups (cleanup);
+       xfree (found_pathname1);
        return NULL;
      }
 
@@ -721,6 +738,7 @@
        gdb_bfd_unref (archive_bfd);
        gdb_bfd_unref (object_bfd);
        do_cleanups (cleanup);
+       xfree (found_pathname1);
        return NULL;
      }
 
@@ -729,7 +747,8 @@
       synthetic name.  Otherwise, we would only be displaying the name
       of the archive member object.  */
    xfree (bfd_get_filename (object_bfd));
-   object_bfd->filename = xstrdup (pathname);
+   strcat (found_pathname1, sep);
+   object_bfd->filename = found_pathname1;
 
    gdb_bfd_unref (archive_bfd);
    do_cleanups (cleanup);

Re: set sysroot command on AIX has no effect.

[Ulrich Weigand]

Sangamesh Mallayya wrote:

> Here is the change i have it right now which takes care of.
> 
> 1) calling solib_find and then solib_bfd_fopen.
> 2) Appending parenthesized member name.

This is looking mostly good.  The one thing I don't like is all the extra
xfree calls.  I think a better way would to not actually xstrdup the found
pathname early after all, but instead just get it from the BFD.  So instead
of the original:

   xfree (bfd_get_filename (object_bfd));
   object_bfd->filename = xstrdup (pathname);

you'd do something like:

   object_bfd->filename = xrealloc (object_bfd->filename, ...);
   strcat (object_bfd->filename, sep);

> +   /* Calling solib_find makes certain that sysroot path is set properly
> +      if program has a dependency on .a archive and sysroot is set via
> +      set sysroot command */

Minor coding style issue: sentences in a comment should end with a '.'
followed by two spaces, like so:
        ... set sysroot command.  */

> +   found_pathname1 = xstrdup (found_pathname);
> +   found_path_len = strlen (found_pathname1) + strlen (sep);
> +   found_pathname1 = xrealloc (found_pathname1, found_path_len);

See comment above, but also: there should be a "+ 1" for the null
terminator in the length computation somewhere.

Bye,
Ulrich

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

Re: set sysroot command on AIX has no effect.

[Ulrich Weigand]

I wrote:

> This is looking mostly good.  The one thing I don't like is all the extra
> xfree calls.  I think a better way would to not actually xstrdup the found
> pathname early after all, but instead just get it from the BFD.  So instead
> of the original:
> 
>    xfree (bfd_get_filename (object_bfd));
>    object_bfd->filename = xstrdup (pathname);
> 
> you'd do something like:
> 
>    object_bfd->filename = xrealloc (object_bfd->filename, ...);
>    strcat (object_bfd->filename, sep);

Sorry, I mixed up the two BFDs here, we need to get the pathname from
the archive_bfd, of coutse.  So this would be something like:

    xfree (bfd_get_filename (object_bfd));
    object_bfd->filename = xstrprintf ("%s%s", bfd_get_filename (archive_bfd), sep);

Bye,
Ulrich

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

Re: set sysroot command on AIX has no effect.

[Sangamesh Mallayya]

Hi Ulrich,

> > This is looking mostly good.  The one thing I don't like is all the 
extra
> > xfree calls.  I think a better way would to not actually xstrdup the 
found
> > pathname early after all, but instead just get it from the BFD.  So 
instead
> > of the original:
> > 
> >    xfree (bfd_get_filename (object_bfd));
> >    object_bfd->filename = xstrdup (pathname);
> > 
> > you'd do something like:
> > 
> >    object_bfd->filename = xrealloc (object_bfd->filename, ...);
> >    strcat (object_bfd->filename, sep);
> 
> Sorry, I mixed up the two BFDs here, we need to get the pathname from
> the archive_bfd, of coutse.  So this would be something like:
> 
>     xfree (bfd_get_filename (object_bfd));
>     object_bfd->filename = xstrprintf ("%s%s", bfd_get_filename 
> (archive_bfd), sep);
> 

Thanks again. Here is the updated patch and looks much better than the 
previous one.

--- ./gdb/solib-aix.c_orig      2016-10-04 03:22:01.000000000 -0500
+++ ./gdb/solib-aix.c   2016-10-13 03:09:26.000000000 -0500
@@ -648,6 +648,8 @@
    char *member_name;
    bfd *archive_bfd, *object_bfd;
    struct cleanup *cleanup;
+   int found_file;
+   char *found_pathname;
 
    if (pathname[path_len - 1] != ')')
      return solib_bfd_open (pathname);
@@ -669,7 +671,13 @@
    member_name = xstrprintf ("%.*s", path_len - filename_len - 2, sep + 
1);
    make_cleanup (xfree, member_name);
 
-   archive_bfd = gdb_bfd_open (filename, gnutarget, -1);
+   /* Calling solib_find makes certain that sysroot path is set properly
+      if program has a dependency on .a archive and sysroot is set via
+      set sysroot command.  */
+   found_pathname = solib_find (filename, &found_file);
+   if (found_pathname == NULL)
+       perror_with_name (pathname);
+   archive_bfd = solib_bfd_fopen (found_pathname, found_file);
    if (archive_bfd == NULL)
      {
        warning (_("Could not open `%s' as an executable file: %s"),
@@ -724,12 +732,14 @@
        return NULL;
      }
 
-   /* Override the returned bfd's name with our synthetic name in order
-      to allow commands listing all shared libraries to display that
-      synthetic name.  Otherwise, we would only be displaying the name
-      of the archive member object.  */
+   /* Override the returned bfd's name with the name returned from 
solib_find
+      along with appended parenthesized member name in order to allow 
commands
+      listing all shared libraries to display. 
+      Otherwise, we would only be displaying the name of the archive 
member
+      object.  */
    xfree (bfd_get_filename (object_bfd));
-   object_bfd->filename = xstrdup (pathname);
+   object_bfd->filename = xstrprintf ("%s%s",
+                                      bfd_get_filename (archive_bfd), 
sep);
 
    gdb_bfd_unref (archive_bfd);
    do_cleanups (cleanup);

I ran gdb.base regression test too and no new failures are seen.
Here is the summary.

# of expected passes            9860
# of unexpected failures        1594
# of expected failures          14
# of unresolved testcases       1
# of untested testcases         60
# of unsupported tests          30

Re: set sysroot command on AIX has no effect.

[Ulrich Weigand]

Sangamesh Mallayya wrote:

> Thanks again. Here is the updated patch and looks much better than the
> previous one.

Indeed, this looks good to me now.  We still need a ChangeLog entry,
but apart from that, this is ready to check in.

> I ran gdb.base regression test too and no new failures are seen.

Thanks!

Bye,
Ulrich

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

Re: set sysroot command on AIX has no effect.

[Sangamesh Mallayya]

[-- Attachment #1: Type: text/plain, Size: 322 bytes --]

Hi Ulrich,


> > Thanks again. Here is the updated patch and looks much better than the
> > previous one.
> 
> Indeed, this looks good to me now.  We still need a ChangeLog entry,
> but apart from that, this is ready to check in.
> 

Please find the ChangeLog & patch file attached.




Thanks,
Sangamesh

[-- Attachment #2: ChangeLog --]
[-- Type: application/octet-stream, Size: 178 bytes --]

	* solib-aix.c (solib_aix_bfd_open): Call solib_find so that sysroot
	path is set properly if program has a dependency on .a archive and
	sysroot is set via set sysroot command.

[-- Attachment #3: sysroot.patch --]
[-- Type: application/octet-stream, Size: 1919 bytes --]

--- ./gdb/solib-aix.c_orig	2016-10-04 03:22:01.000000000 -0500
+++ ./gdb/solib-aix.c	2016-10-13 03:09:26.000000000 -0500
@@ -648,6 +648,8 @@
    char *member_name;
    bfd *archive_bfd, *object_bfd;
    struct cleanup *cleanup;
+   int found_file;
+   char *found_pathname;
  
    if (pathname[path_len - 1] != ')')
      return solib_bfd_open (pathname);
@@ -669,7 +671,13 @@
    member_name = xstrprintf ("%.*s", path_len - filename_len - 2, sep + 1);
    make_cleanup (xfree, member_name);
  
-   archive_bfd = gdb_bfd_open (filename, gnutarget, -1);
+   /* Calling solib_find makes certain that sysroot path is set properly
+      if program has a dependency on .a archive and sysroot is set via
+      set sysroot command.  */
+   found_pathname = solib_find (filename, &found_file);
+   if (found_pathname == NULL)
+       perror_with_name (pathname);
+   archive_bfd = solib_bfd_fopen (found_pathname, found_file);
    if (archive_bfd == NULL)
      {
        warning (_("Could not open `%s' as an executable file: %s"),
@@ -724,12 +732,14 @@
        return NULL;
      }
  
-   /* Override the returned bfd's name with our synthetic name in order
-      to allow commands listing all shared libraries to display that
-      synthetic name.  Otherwise, we would only be displaying the name
-      of the archive member object.  */
+   /* Override the returned bfd's name with the name returned from solib_find
+      along with appended parenthesized member name in order to allow commands
+      listing all shared libraries to display. 
+      Otherwise, we would only be displaying the name of the archive member
+      object.  */
    xfree (bfd_get_filename (object_bfd));
-   object_bfd->filename = xstrdup (pathname);
+   object_bfd->filename = xstrprintf ("%s%s",
+                                      bfd_get_filename (archive_bfd), sep);
  
    gdb_bfd_unref (archive_bfd);
    do_cleanups (cleanup);

Re: set sysroot command on AIX has no effect.

[Ulrich Weigand]

Sangamesh Mallayya wrote:

> > > Thanks again. Here is the updated patch and looks much better than the
> > > previous one.
> > Indeed, this looks good to me now.  We still need a ChangeLog entry,
> > but apart from that, this is ready to check in.
> 
> Please find the ChangeLog & patch file attached.

I've pushed this to mainline now.

Note that the patch didn't cleanly apply before I manually fixed some
white space issues.  I find the simplest way to create patches that
are good to apply (and contain proper commit messages etc.) is to work
in a local git repo and use "git format-patch".

Thanks,
Ulrich

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