[PATCH] Modify align-c/align-c++ test case for AIX

[PATCH] Modify align-c/align-c++ test case for AIX

[Aditya Kamath1]

[-- Attachment #1.1: Type: text/plain, Size: 974 bytes --]

Hi all,

Please find attached the patch. {See:- 0001-Modify-align-c-align-c-test-case-for-AIX.patch}

In AIX, we have observed around 1500 failures in align-c/align-c++ testcase. The reason being this testcase shows the alignment outputs of global variables that are unused in the main (). In AIX we need to use atleast one global variable to access the same. This patch is a fix to the same. This issue is like this link pasted in the next line which we fixed before. https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60204874f5a987b164f7f194d3f96729847fe329

Kindly let me know if this is okay.

Test case command:- gmake RUNTESTFLAGS="gdb.base/align-c.exp" check
                                       gmake RUNTESTFLAGS="gdb.base/align-c++.exp" check
Test case passes before the patch :- 0 for align-c++ and align-c
Test case passes after this patch:- 1134 for align-c++ and 406 for align-c.

Have a nice day ahead.

Thanks and regards,
Aditya.

[-- Attachment #2: 0001-Modify-align-c-align-c-test-case-for-AIX.patch --]
[-- Type: application/octet-stream, Size: 1004 bytes --]

From 5daed42ecd34d0019f3a2e4828a5b014ee865452 Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
Date: Fri, 10 Mar 2023 01:22:14 -0600
Subject: [PATCH] Modify align-c/align-c++ test case for AIX

On AIX, the debugger cannot access global variables before they
they are used in any function. Hence we add this small change so
that we can test the same in AIX.
---
 gdb/testsuite/gdb.base/align.exp.tcl | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gdb/testsuite/gdb.base/align.exp.tcl b/gdb/testsuite/gdb.base/align.exp.tcl
index 6a75a14d887..f18243080ce 100644
--- a/gdb/testsuite/gdb.base/align.exp.tcl
+++ b/gdb/testsuite/gdb.base/align.exp.tcl
@@ -109,6 +109,11 @@ proc prepare_test_source_file { lang } {
     # Epilogue.
     puts $outfile {
 	int main() {
+	    #ifdef _AIX
+	    /* On AIX, the debugger cannot access global variables before they
+		they are used in the functions.  */
+		a_char_x_char++;
+	    #endif
 	    return 0;
 	}
     }
-- 
2.38.3

Re: [PATCH] Modify align-c/align-c++ test case for AIX

[Tom Tromey]

>>>>> Aditya Kamath1 via Gdb-patches <gdb-patches@sourceware.org> writes:

> In AIX, we have observed around 1500 failures in align-c/align-c++
> testcase. The reason being this testcase shows the alignment outputs
> of global variables that are unused in the main (). In AIX we need to
> use atleast one global variable to access the same.

Could you say why this is needed?

Tom

RE: [PATCH] Modify align-c/align-c++ test case for AIX

[Aditya Kamath1]

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

Hi Tom, Ulrich and community,

>> In AIX, we have observed around 1500 failures in align-c/align-c++
>> testcase. The reason being this testcase shows the alignment outputs
>> of global variables that are unused in the main (). In AIX we need to
>> use atleast one global variable to access the same.

> Could you say why this is needed?

Consider the code:-
#include <stdio.h>
int global_variable = 1;
int main(){
        int local_variable = 1;
        printf ("Simple print statement \n");
        return 0;
}

When we debug with gdb and try to print global_variable this is what happens:-
(gdb) b main
Breakpoint 1 at 0x1000052c: file /home/aditya/gdb_tests/simple_test.c, line 4.
(gdb) r
Starting program: /home/aditya/gdb_tests/simple_test
Breakpoint 1, main () at /home/aditya/gdb_tests/simple_test.c:4
4               int local_variable = 1;
(gdb) p global_variable
ret = -1
ret = -1
Cannot access memory at address 0xffffffff

This ret = -1 is a printf(“ret = %d\n”, ret) statement I have used within my rs6000-aix-nat.c code to check what my ptrace call returns..

So what happens is ptrace call in AIX is not allowed to access any variable or rather say memory or  a segment which is not used in the user/debuggee code. Hence ptrace returned -1 and therefore the address 0xffffffff is used which we will not be able to access anyway.

Now Consider the code where we use it:-
#include <stdio.h>
int global_variable = 1;
int main(){
        int local_variable = 1;
        global_variable++;
        printf ("Simple print statement \n");
        return 0;
}

(gdb) b main
Breakpoint 1 at 0x1000052c: file /home/aditya/gdb_tests/simple_test.c, line 4.
(gdb) r
Starting program: /home/aditya/gdb_tests/simple_test
Breakpoint 1, main () at /home/aditya/gdb_tests/simple_test.c:4
4               int local_variable = 1;

(gdb) p global_variable
ret = 1
$1 = 1
(gdb)

This is because now we have the main () using the global_variable and hence ptrace got the access for the same and could return the correct memory address.

The key thing is user functions need to use global variables to get access. Otherwise the ptrace calls will return -1 and hence in our align-c/align-c++ all the testcases failed without the patch and the moment I used one global variable ptrace calls got access and the test cases passed.

Kindly let me know if you and the community think the patch in the previous email was therefore the right thing to do.

Have a nice day ahead.

Thanks and regards,
Aditya.

From: Tom Tromey <tom@tromey.com>
Date: Friday, 10 March 2023 at 8:18 PM
To: Aditya Kamath1 via Gdb-patches <gdb-patches@sourceware.org>
Cc: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>, Aditya Kamath1 <Aditya.Kamath1@ibm.com>, Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: [EXTERNAL] Re: [PATCH] Modify align-c/align-c++ test case for AIX
>>>>> Aditya Kamath1 via Gdb-patches <gdb-patches@sourceware.org> writes:

> In AIX, we have observed around 1500 failures in align-c/align-c++
> testcase. The reason being this testcase shows the alignment outputs
> of global variables that are unused in the main (). In AIX we need to
> use atleast one global variable to access the same.

Could you say why this is needed?

Tom

Re: [PATCH] Modify align-c/align-c++ test case for AIX

[Ulrich Weigand]

Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>(gdb) p global_variable
>ret = -1
>ret = -1
>Cannot access memory at address 0xffffffff
>
>This ret = -1 is a printf(“ret = %d\n”, ret) statement I have 
>used within my rs6000-aix-nat.c code to check what my ptrace call returns.. 

I don't think the problem is the ptrace call itself, it looks
more like that the *address* is incorrect.

It would be good to understand a bit better why this is happening.
I thought on AIX global variables should reside in the TOC section,
so I thought we should be able to get their address ...

Can you look into more detail how the address of the
global_variable symbol is being determined, and why
we end up with an invalid value (0xffffffff)?


Bye,
Ulrich

Re: [PATCH] Modify align-c/align-c++ test case for AIX

[Aditya Kamath1]

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

Hi Ulrich, Tom and community,

>I don't think the problem is the ptrace call itself, it looks
>more like that the *address* is incorrect.

>It would be good to understand a bit better why this is happening.
>I thought on AIX global variables should reside in the TOC section,
>so I thought we should be able to get their address ...

>Can you look into more detail how the address of the
>global_variable symbol is being determined, and why
>we end up with an invalid value (0xffffffff)?

So my understanding is as follows. When we run the p global_variable command we call functions in the order print_command-> print_command_1-> print_value-> value::record_latest-> value::fetch_lazy-> value::fetch_lazy_memory-> read_value_memory…

In read_value_memory () we call target_xfer_partial() where we come rs6000-aix-target’s xfer partial. Since the read_buf value is set we call the ptrace call

buffer.word = rs6000_ptrace32 (PT_READ_I, pid,
                                             (int *)(uintptr_t)rounded_offset,
                                             0, NULL);

This set errno to 5.

Then the following value is returned.
if (errno)
              return TARGET_XFER_EOF;

Then we go to read_value_memory () where these lines execute
else if (status == TARGET_XFER_EOF)
        memory_error (TARGET_XFER_E_IO, memaddr + xfered_total);

In memory_error () we hit err=TARGET_XFER_E_IO in the switch case and print cannot access memory at 0xffffff.

>why
>we end up with an invalid value (0xffffffff)?

After we come out of xfer_partial from rs6000-aix-nat.c in read_value_memory () since we got TARGET_XFER_EOF so it executes

“else if (status == TARGET_XFER_EOF)
        memory_error (TARGET_XFER_E_IO, memaddr + xfered_total);”

This ( memaddr + xfered_total ) is responsible for it. Memaddr was 18446744073709551615. This is itself FFFFFFFFFFFFFFFF.

This got assigned at CORE_ADDR addr = address (); in value::fetch_lazy_memory ()..

In the CORE_ADDR
value::address ()

the return value
is m_location.address + m_offset = -1 and that is 0xffffff in 2’s complement.

So this is what is happening.

Kindly let me know what you think about this and also about the fix I suggested to the align test case with the patch attached in the previous email sent by me.

Thanks and regards,
Aditya.


From: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
Date: Monday, 13 March 2023 at 7:40 PM
To: gdb-patches@sourceware.org <gdb-patches@sourceware.org>, Aditya Kamath1 <Aditya.Kamath1@ibm.com>, tom@tromey.com <tom@tromey.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: Re: [PATCH] Modify align-c/align-c++ test case for AIX
Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>(gdb) p global_variable
>ret = -1
>ret = -1
>Cannot access memory at address 0xffffffff
>
>This ret = -1 is a printf(“ret = %d\n”, ret) statement I have
>used within my rs6000-aix-nat.c code to check what my ptrace call returns..

I don't think the problem is the ptrace call itself, it looks
more like that the *address* is incorrect.

It would be good to understand a bit better why this is happening.
I thought on AIX global variables should reside in the TOC section,
so I thought we should be able to get their address ...

Can you look into more detail how the address of the
global_variable symbol is being determined, and why
we end up with an invalid value (0xffffffff)?


Bye,
Ulrich

Re: [PATCH] Modify align-c/align-c++ test case for AIX

[Tom Tromey]

>>>>> Aditya Kamath1 <Aditya.Kamath1@ibm.com> writes:

> In the CORE_ADDR
> value::address ()
> the return value 
> is m_location.address + m_offset = -1 and that is 0xffffff in 2’s complement. 

I think the question is why is it -1 here.
Like, is it wrong in the DWARF?  Or did the DWARF reader do something
wrong?  Or was some runtime offset incorrectly applied?  Etc.

Tom

RE: [PATCH] Modify align-c/align-c++ test case for AIX

[Aditya Kamath1]

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

Hi Tom, Ulrich and community,

>> In the CORE_ADDR
>> value::address ()
>> the return value
>> is m_location.address + m_offset = -1 and that is 0xffffff in 2’s complement.

>I think the question is why is it -1 here.
>Like, is it wrong in the DWARF?  Or did the DWARF reader do something
>wrong?  Or was some runtime offset incorrectly applied?  Etc.

The problem is from the DWARF itself. Our GDB code is picking up things correctly. Specifically when we do not use the global variables or the global variable section it those variables are assigned 0xffffff by the linker. We give -g -gdwarf when we compile.. So here is my explaination.

Consider the code
bash-5.1$ cat ~/gdb_tests/simple_test.c
#include <stdio.h>
int global_variable1 = 1;
int global_variable2 = 2;
int main(){
        int local_variable = 1;
        global_variable2++;
        printf ("Simple print statement \n");
        return 0;
}

So I checked the code. DWARF values are being read correctly and that address is exactly passed into the code. The dwarf dump output in this case is as pasted below. Kindly note we use global variables.

<1><  347>      DW_TAG_variable
                DW_AT_name                  global_variable1
                DW_AT_decl_file             2
                DW_AT_decl_line             2
                DW_AT_decl_column           5
                DW_AT_type                  <175>
                DW_AT_external              yes
                DW_AT_location              DW_OP_addr 0x20000e80
<1><  378>      DW_TAG_variable
                DW_AT_name                  global_variable2
                DW_AT_decl_file             2
                DW_AT_decl_line             3
                DW_AT_decl_column           5
                DW_AT_type                  <175>
                DW_AT_external              yes
                DW_AT_location              DW_OP_addr 0x20000e84

GDB output for this case:-

bash-5.1$ ./gdb ~/gdb_tests/simple_test
GNU gdb (GDB) 14.0.50.20230221-git
Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "powerpc64-ibm-aix7.2.0.0".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
https://www.gnu.org/software/gdb/bugs/.
Find the GDB manual and other documentation resources online at:
    http://www.gnu.org/software/gdb/documentation/.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /home/aditya/gdb_tests/simple_test...
(gdb) b main
Breakpoint 1 at 0x1000052c: file /home/aditya/gdb_tests/simple_test.c, line 5.
(gdb) r
Starting program: /home/aditya/gdb_tests/simple_test

Breakpoint 1, main () at /home/aditya/gdb_tests/simple_test.c:5
5               int local_variable = 1;
(gdb) p global_variable1
$1 = 1
(gdb) p global_variable2
$2 = 2
(gdb)

Now kindly consider the code
bash-5.1$ cat ~/gdb_tests/simple_test.c
#include <stdio.h>
int global_variable1 = 1;
int global_variable2 = 2;
int main(){
        int local_variable = 1;
        printf ("Simple print statement \n");
        return 0;
}

The dwarf dump output for the same..
<1><  347>      DW_TAG_variable
                DW_AT_name                  global_variable1
                DW_AT_decl_file             2
                DW_AT_decl_line             2
                DW_AT_decl_column           5
                DW_AT_type                  <175>
                DW_AT_external              yes
                DW_AT_location              DW_OP_addr 0xffffffff
<1><  378>      DW_TAG_variable
                DW_AT_name                  global_variable2
                DW_AT_decl_file             2
                DW_AT_decl_line             3
                DW_AT_decl_column           5
                DW_AT_type                  <175>
                DW_AT_external              yes
                DW_AT_location              DW_OP_addr 0xffffffff

The GDB output for the above dwarf dump..

bash-5.1$ ./gdb ~/gdb_tests/simple_test
GNU gdb (GDB) 14.0.50.20230221-git
Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "powerpc64-ibm-aix7.2.0.0".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
https://www.gnu.org/software/gdb/bugs/.
Find the GDB manual and other documentation resources online at:
    http://www.gnu.org/software/gdb/documentation/.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /home/aditya/gdb_tests/simple_test...
(gdb) b main
Breakpoint 1 at 0x1000052c: file /home/aditya/gdb_tests/simple_test.c, line 5.
(gdb) r
Starting program: /home/aditya/gdb_tests/simple_test

Breakpoint 1, main () at /home/aditya/gdb_tests/simple_test.c:5
5               int local_variable = 1;
(gdb) p global_variable1
Cannot access memory at address 0xffffffff
(gdb)

So in the dwarf dump we see that in case of ‘n’ number of global variables declared and ‘n’ unused global variables then all variables are assigned 0xfffffff.

In case of  ‘n’ number of global variables declared and ‘m’ used global variables, where m >= 1 and m <= n, all of them have assigned addresses in AIX.

Kindly let me know what you think of this and the patch we sent.

Have a nice day ahead.

Thanks and regards,
Aditya.

From: Tom Tromey <tom@tromey.com>
Date: Wednesday, 15 March 2023 at 6:15 PM
To: Aditya Kamath1 <Aditya.Kamath1@ibm.com>
Cc: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>, gdb-patches@sourceware.org <gdb-patches@sourceware.org>, tom@tromey.com <tom@tromey.com>, Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: [EXTERNAL] Re: [PATCH] Modify align-c/align-c++ test case for AIX
>>>>> Aditya Kamath1 <Aditya.Kamath1@ibm.com> writes:

> In the CORE_ADDR
> value::address ()
> the return value
> is m_location.address + m_offset = -1 and that is 0xffffff in 2’s complement.

I think the question is why is it -1 here.
Like, is it wrong in the DWARF?  Or did the DWARF reader do something
wrong?  Or was some runtime offset incorrectly applied?  Etc.

Tom

Re: [PATCH] Modify align-c/align-c++ test case for AIX

[Ulrich Weigand]

Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:
>>I think the question is why is it -1 here.
>>Like, is it wrong in the DWARF?  Or did the DWARF reader do something
>>wrong?  Or was some runtime offset incorrectly applied?  Etc.
> 
>The problem is from the DWARF itself. Our GDB code is picking up things correctly.
>Specifically when we do not use the global variables or the global variable
>section it those variables are assigned 0xffffff by the linker.

So is this some linker optimization where those global variables
are just removed if they are not used?  Are the variables still
present in the *symbol* table (not DWARF)?

If it is a linker optimization, maybe this can be switched off
via some command line option?

If not, then I guess something like your original patch does
seem a reasonable solution ...

Bye,
Ulrich

Re: [PATCH] Modify align-c/align-c++ test case for AIX

[Tom Tromey]

>>>>> Aditya Kamath1 <Aditya.Kamath1@ibm.com> writes:

>                 DW_AT_location              DW_OP_addr 0xffffffff

In addition to what Ulrich said, I wonder if this behavior should have a
workaround in the gdb DWARF reader.  E.g., gdb could use producer
sniffing to detect when this might happen, and then mark these symbols
as optimized out, or something along those lines.

Tom

RE: [PATCH] Modify align-c/align-c++ test case for AIX

[Aditya Kamath1]

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

Hi Ulrich, Tom and Community,

>So is this some linker optimization where those global variables
>are just removed if they are not used?  Are the variables still
>present in the *symbol* table (not DWARF)?

>If it is a linker optimization, maybe this can be switched off
>via some command line option?

Yes Ulrich. It is a linker optimisation.
So we need to use these options while compiling.

bash-5.1$ gcc -g -gdwarf ~/gdb_tests/simple_test.c -o ~/gdb_tests/simple_test -Wl,-bnogc  -lodm -lcfg
bash-5.1$ cat ~/gdb_tests/simple_test.c
#include <stdio.h>
int global_variable1 = 1;
int global_variable2 = 2;
int main(){
        int local_variable = 1;
        printf ("Simple print statement \n");
        return 0;
}
bash-5.1$
bash-5.1$ ./gdb ~/gdb_tests/simple_test
GNU gdb (GDB) 14.0.50.20230221-git
Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "powerpc64-ibm-aix7.2.0.0".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
https://www.gnu.org/software/gdb/bugs/.
Find the GDB manual and other documentation resources online at:
    http://www.gnu.org/software/gdb/documentation/.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /home/aditya/gdb_tests/simple_test...
(gdb) p global_variable1
$1 = 1
(gdb)

We are able to access it now.
Dwarf dump output of the above debug code.
<1><  347>      DW_TAG_variable
                DW_AT_name                  global_variable1
                DW_AT_decl_file             2
                DW_AT_decl_line             2
                DW_AT_decl_column           5
                DW_AT_type                  <175>
                DW_AT_external              yes
                DW_AT_location              DW_OP_addr 0x20000570
<1><  378>      DW_TAG_variable
                DW_AT_name                  global_variable2
                DW_AT_decl_file             2
                DW_AT_decl_line             3
                DW_AT_decl_column           5
                DW_AT_type                  <175>
                DW_AT_external              yes
                DW_AT_location              DW_OP_addr 0x20000574

If we do not use the options -Wl,-bnogc  -lodm -lcfg during compilation , then the dwarf dump is as shown below.

bash-5.1$ gcc -g -gdwarf ~/gdb_tests/simple_test.c -o ~/gdb_tests/simple_test
bash-5.1$ ~/dwarfdump -a ~/gdb_tests/simple_test

.debug_info
……….
<1><  347>      DW_TAG_variable
                DW_AT_name                  global_variable1
                DW_AT_decl_file             2
                DW_AT_decl_line             2
                DW_AT_decl_column           5
                DW_AT_type                  <175>
                DW_AT_external              yes
                DW_AT_location              DW_OP_addr 0xffffffff
<1><  378>      DW_TAG_variable
                DW_AT_name                  global_variable2
                DW_AT_decl_file             2
                DW_AT_decl_line             3
                DW_AT_decl_column           5
                DW_AT_type                  <175>
                DW_AT_external              yes
                DW_AT_location              DW_OP_addr 0xffffffff
<1><  409>      DW_TAG_subprogram
                DW_AT_external              yes
                DW_AT_name                  main
                DW_AT_decl_file             2
                DW_AT_decl_line             4
                DW_AT_decl_column           5
                DW_AT_type                  <175>
                DW_AT_low_pc                0x10000518
                DW_AT_high_pc               92
                DW_AT_frame_base            <expr length 0>

So clearly, linker optimises out unused global variables. More about the same can be read in the documentation {https://www.ibm.com/docs/en/aix/7.1?topic=l-ld-command }

So I was thinking if we can pass these to our site.exp file if the target is AIX:

set CC_FOR_TARGET "/opt/freeware/bin/gcc"
set CXX_FOR_TARGET "/opt/freeware/bin/g++"
set CXXFLAGS_FOR_TARGET "-O0 -w -g -gdwarf -maix64"
set CFLAGS_FOR_TARGET "-O0 -w -g -gdwarf -maix64"

with the options -Wl,-bnogc  -lodm -lcfg.

We also use -maix32 for testing in 32 bit mode as well.

I think this would be a better patch and in AIX we will not be seeing so many failures as we see now instead of the initial proposed ifdef patch. Only thing is we need to add this in a file which I am not aware of such that if the target is AIX we need to add these flags in the site.exp file so that while running test cases we get this file to pass these flags during the compilation of them.

Since I am a beginner to the same I need your guidance and expertise here. I wish to fix this for AIX and GDB community. Currently I manually add these flags which is not correct. Which MakeFile or a gdb.exp file can I do the same? Kindly let me know. Also let me know what you think of the explaination.

>>                 DW_AT_location              DW_OP_addr 0xffffffff

>In addition to what Ulrich said, I wonder if this behavior should have a
>workaround in the gdb DWARF reader.  E.g., gdb could use producer
>sniffing to detect when this might happen, and then mark these symbols
>as optimized out, or something along those lines.

Hi Tom,

Thank you for this suggestion. So I was thinking to spilt this patch into two.

One for the passing of flags in AIX for the site.exp file in testsuite.

Another to tell the AIX users that “You have not passed the flags and hence the linker has optimised out this variables, kindly compile with the same”.  Your idea is nice to tell our users, since many of AIX folks will not be aware of the linker optimisation.

Can you or anyone in the community suggest me an example in the GDB code that uses this producer sniffing we can do the same.

Kindly let me know.

Have a nice day ahead.

Thanks and regards,
Aditya.

From: Tom Tromey <tom@tromey.com>
Date: Saturday, 18 March 2023 at 3:33 AM
To: Aditya Kamath1 <Aditya.Kamath1@ibm.com>
Cc: Tom Tromey <tom@tromey.com>, Ulrich Weigand <Ulrich.Weigand@de.ibm.com>, gdb-patches@sourceware.org <gdb-patches@sourceware.org>, Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>, simon.marchi@efficios.com <simon.marchi@efficios.com>
Subject: [EXTERNAL] Re: [PATCH] Modify align-c/align-c++ test case for AIX
>>>>> Aditya Kamath1 <Aditya.Kamath1@ibm.com> writes:

>                 DW_AT_location              DW_OP_addr 0xffffffff

In addition to what Ulrich said, I wonder if this behavior should have a
workaround in the gdb DWARF reader.  E.g., gdb could use producer
sniffing to detect when this might happen, and then mark these symbols
as optimized out, or something along those lines.

Tom

Re: [PATCH] Modify align-c/align-c++ test case for AIX

[Ulrich Weigand]

Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>So clearly, linker optimises out unused global variables. More about the
>same can be read in the documentation
>{https://www.ibm.com/docs/en/aix/7.1?topic=l-ld-command } 

I see the -Wl,-bnogc option there, this makes sense.

I'm not sure why "-lodm -lcfg" should be necessary as well, this
doesn't really appear related to me.  Can you try without those?


>So I was thinking if we can pass these to our site.exp file if the target is AIX:
>
>set CC_FOR_TARGET "/opt/freeware/bin/gcc"
>set CXX_FOR_TARGET "/opt/freeware/bin/g++"
>set CXXFLAGS_FOR_TARGET "-O0 -w -g -gdwarf -maix64"
>set CFLAGS_FOR_TARGET "-O0 -w -g -gdwarf -maix64"
>
>with the options -Wl,-bnogc  -lodm -lcfg. 
>
>We also use -maix32 for testing in 32 bit mode as well. 
>
>I think this would be a better patch and in AIX we will not be seeing
>so many failures as we see now instead of the initial proposed ifdef patch.
>Only thing is we need to add this in a file which I am not aware of such
>that if the target is AIX we need to add these flags in the site.exp
>file so that while running test cases we get this file to pass these flags
>during the compilation of them. 

If the -bnogc flag is always necessary with the AIX linker, this
should be handled automatically by the test instead of via site.exp.

I'm assuming this is only needed with test files that use global
variables unused in the source code, but used by the debugger.
I think it would be best to explicitly add the flag to those tests
where it is needed.

For example, for the align-c tests, you can change the beginning
of run_alignment_test in gdb.base/align.exp.tcl to something like:

    set flags {debug}
    if { "$lang" == "c++" } {
        lappend flags "additional_flags=-std=c++11"
    }
    if { [istarget *-*-aix*] } {
        # Disable linker garbage collection to avoid optimizing
        # out global variables unused in the test source code.
        lappend flags "ldflags=-Wl,-bnogc"
    }
    standard_testfile $filename
    if {[prepare_for_testing "failed to prepare" "$testfile" $srcfile $flags]} {
        return -1
    }

Bye,
Ulrich

Re: [PATCH] Modify align-c/align-c++ test case for AIX

[Pedro Alves]

On 2023-03-21 7:41 a.m., Ulrich Weigand via Gdb-patches wrote:
> Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:
> 
>> So clearly, linker optimises out unused global variables. More about the
>> same can be read in the documentation
>> {https://www.ibm.com/docs/en/aix/7.1?topic=l-ld-command } 
> 
> I see the -Wl,-bnogc option there, this makes sense.
> 
> I'm not sure why "-lodm -lcfg" should be necessary as well, this
> doesn't really appear related to me.  Can you try without those?
> 
> 
>> So I was thinking if we can pass these to our site.exp file if the target is AIX:
>>
>> set CC_FOR_TARGET "/opt/freeware/bin/gcc"
>> set CXX_FOR_TARGET "/opt/freeware/bin/g++"
>> set CXXFLAGS_FOR_TARGET "-O0 -w -g -gdwarf -maix64"
>> set CFLAGS_FOR_TARGET "-O0 -w -g -gdwarf -maix64"
>>
>> with the options -Wl,-bnogc  -lodm -lcfg. 
>>
>> We also use -maix32 for testing in 32 bit mode as well. 
>>
>> I think this would be a better patch and in AIX we will not be seeing
>> so many failures as we see now instead of the initial proposed ifdef patch.
>> Only thing is we need to add this in a file which I am not aware of such
>> that if the target is AIX we need to add these flags in the site.exp
>> file so that while running test cases we get this file to pass these flags
>> during the compilation of them. 
> 
> If the -bnogc flag is always necessary with the AIX linker, this
> should be handled automatically by the test instead of via site.exp.
> 
> I'm assuming this is only needed with test files that use global
> variables unused in the source code, but used by the debugger.
> I think it would be best to explicitly add the flag to those tests
> where it is needed.

I'm wondering whether __attribute__((used)) on the global variable makes any
difference.  IIRC with LLVM it prevents link-time stripping on Mach-O and PE/COFF,
but I don't know about XCOFF + GCC.  (It doesn't on ELF, you
need 'retain' for that.)

Pedro Alves

> 
> For example, for the align-c tests, you can change the beginning
> of run_alignment_test in gdb.base/align.exp.tcl to something like:
> 
>     set flags {debug}
>     if { "$lang" == "c++" } {
>         lappend flags "additional_flags=-std=c++11"
>     }
>     if { [istarget *-*-aix*] } {
>         # Disable linker garbage collection to avoid optimizing
>         # out global variables unused in the test source code.
>         lappend flags "ldflags=-Wl,-bnogc"
>     }
>     standard_testfile $filename
>     if {[prepare_for_testing "failed to prepare" "$testfile" $srcfile $flags]} {
>         return -1
>     }
> 
> Bye,
> Ulrich
> 

Re: [PATCH] Modify align-c/align-c++ test case for AIX

[Tom Tromey]

>>>>> "Ulrich" == Ulrich Weigand via Gdb-patches <gdb-patches@sourceware.org> writes:

Ulrich> If the -bnogc flag is always necessary with the AIX linker, this
Ulrich> should be handled automatically by the test instead of via site.exp.

Ulrich> I'm assuming this is only needed with test files that use global
Ulrich> variables unused in the source code, but used by the debugger.
Ulrich> I think it would be best to explicitly add the flag to those tests
Ulrich> where it is needed.

I think there's also cases in the test suite where we've added uses of
variables to ensure they aren't optimized away.  With a comment
somewhere explaining why, I'm also ok with that.  It seems less
intrusive maybe than adding ldflags to a lot of tests.

Tom

Re: [PATCH] Modify align-c/align-c++ test case for AIX

[Ulrich Weigand]

Tom Tromey <tom@tromey.com> wrote:
>Ulrich> If the -bnogc flag is always necessary with the AIX linker, this
>Ulrich> should be handled automatically by the test instead of via site.exp.
>
>Ulrich> I'm assuming this is only needed with test files that use global
>Ulrich> variables unused in the source code, but used by the debugger.
>Ulrich> I think it would be best to explicitly add the flag to those tests
>Ulrich> where it is needed.
>
>I think there's also cases in the test suite where we've added uses of
>variables to ensure they aren't optimized away.  With a comment
>somewhere explaining why, I'm also ok with that.  It seems less
>intrusive maybe than adding ldflags to a lot of tests.

Well, that was Aditya's original proposal, I guess ...   I'd be fine
with that as well - whatever is easier.

I think doing a global run with -Wl,-bnogc would still be interesting,
simply to get a list of all the tests that would need changing.

Bye,
Ulrich

Re: [PATCH] Modify align-c/align-c++ test case for AIX

[Aditya Kamath1]

[-- Attachment #1.1: Type: text/plain, Size: 8552 bytes --]

Hi Ulrich, Tom, Pedro and community members,

Appreciate your patience and feedback. I had to check a few things before we go ahead. Sorry for the delay in response.

>I see the -Wl,-bnogc option there, this makes sense.

>I'm not sure why "-lodm -lcfg" should be necessary as well, this
>doesn't really appear related to me.  Can you try without those?

So here is the problem.

bash-5.1$ gcc -g -gdwarf ~/gdb_tests/simple_test.c -o ~/gdb_tests/simple_test -Wl,-bnogc
ld: 0711-317 ERROR: Undefined symbol: .odm_initialize
ld: 0711-317 ERROR: Undefined symbol: CuDv_CLASS
ld: 0711-317 ERROR: Undefined symbol: .odm_get_list
ld: 0711-317 ERROR: Undefined symbol: .odm_free_list
ld: 0711-317 ERROR: Undefined symbol: .getattr
ld: 0711-317 ERROR: Undefined symbol: .odm_terminate
ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.
collect2: error: ld returned 8 exit status

When we say no garbage collection, the linker shows these errors. These are garbage symbols. Nowhere in my code pasted below I have used any odm modules symbol. But since the linker is not collecting garbage, this seems to be having symbols that needs to link with ODM and that is not a desired behaviour. So I was using -lodm, -lcfg. But this should not be the case. Another problem I have seen is when we try to link with other modules like -lm {Math} or -lpthread {pthread}, we see similar issues.

The code :-

bash-5.1$ cat ~/gdb_tests/simple_test.c
#include <stdio.h>
int __attribute__((unused)) global_variable1 = 1;
int global_variable2 = 2;
int main(){
        int local_variable = 1;
        printf ("Simple print statement \n");
        return 0;
}

Since this is an inconsistent behaviour while using the -Wl,-bnogc I have decided not to use the same. We do not want the linker to allow any garbage symbols just because we gave an option to it to not collect garbage.

Instead in the documentation { https://www.ibm.com/docs/en/aix/7.1?topic=l-ld-command } I have seen one more option which is more specific.

For the same code above say we compile using the command
bash-5.1$ gcc -g -gdwarf ~/gdb_tests/simple_test.c -o ~/gdb_tests/simple_test -Wl,-uglobal_variable1
bash-5.1$ ./gdb ~/gdb_tests/simple_test
GNU gdb (GDB) 14.0.50.20230327-git
Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "powerpc64-ibm-aix7.2.0.0".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
https://www.gnu.org/software/gdb/bugs/.
Find the GDB manual and other documentation resources online at:
    http://www.gnu.org/software/gdb/documentation/.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /home/aditya/gdb_tests/simple_test...
(gdb) p global_variable1
$1 = 1
(gdb) p global_variable2
$2 = 2
(gdb)

So this -u option “-uSymbolName” tells the linker to only allow variable with name SymbolName or any variable used in this code only. This can be a much better option for us in this testcase than -Wl,bnogc which is collecting any random symbol which garbage. Please find attached a patch {See: 0001-Modify-align-c-align-c-test-case-for-AIX_v2.patch}. I have Tested this and it worked.

Test case passes after patch :- align-c = 406 align-c++ = 1134
Test case passes before patch:- align-c = 0  align-c++ = 0

Partial Dwarf dump output for the same code above

<1><  357>      DW_TAG_variable
                DW_AT_name                  global_variable1
                DW_AT_decl_file             2
                DW_AT_decl_line             2
                DW_AT_decl_column           29
                DW_AT_type                  <185>
                DW_AT_external              yes
                DW_AT_location              DW_OP_addr 0x20000ea0
<1><  388>      DW_TAG_variable
                DW_AT_name                  global_variable2
                DW_AT_decl_file             2
                DW_AT_decl_line             3
                DW_AT_decl_column           5
                DW_AT_type                  <185>
                DW_AT_external              yes
                DW_AT_location              DW_OP_addr 0x20000ea4

This is one option..

>I'm assuming this is only needed with test files that use global
>variables unused in the source code, but used by the debugger.
>I think it would be best to explicitly add the flag to those tests
>where it is needed.

Yes. Your assumption is absolutely correct Ulrich. If unused and no garbage collection option given then in my previous mails dwarf dump output we can see linker adds 0xfffffff as the address of garbage variables.

>I'm wondering whether __attribute__((used)) on the global variable makes any
>difference.  IIRC with LLVM it prevents link-time stripping on Mach-O and PE/COFF,
>but I don't know about XCOFF + GCC.  (It doesn't on ELF, you
>need 'retain' for that.)

Hi Pedro. So thanks for this suggestion. I learnt something new from you. But this __attribute__((used)) did not make any difference in our environment.

>With a comment
>somewhere explaining why, I'm also ok with that.  It seems less
>intrusive maybe than adding ldflags to a lot of tests.

>Well, that was Aditya's original proposal, I guess ...   I'd be fine
>with that as well - whatever is easier.

Hi Ulrich and Tom. Yes, so the second option we have is to do this. I have attached my original patch proposal as well. Please find attached. {See: 0001-Modify-align-c-align-c-test-case-for-AIX_v1.patch}.

So whatever you feel is right from the community angle we should go with that. But yeah, if we use {See: 0001-Modify-align-c-align-c-test-case-for-AIX_v2.patch} we know why we are doing so and test case will look more like our Linux folks.

>I think doing a global run with -Wl,-bnogc would still be interesting,
>simply to get a list of all the tests that would need changing.

So I can tell you without these options what is going on.
Currently in the gdb.base test suite we pass 87%  { 27139  passes and 3918 FAILS } tests in AIX, with the major failures in
align-c -• 412 failures
align-c++ • 1146 failures
whatis-ptype • 1062 failures
attach • 40 failures..

In gdb.cp as well we have 96% tests passed in AIX, with major failure in
align.exp and next-overthrow.exp • 50 failures each..

All these failures are expected to reduce with this fix. Have given numbers with the patch for align-c and align-c++. Not sure of attach and whatis-pytpe. I don’t think that has to do with this patch. Will see what is going on in those two test cases and come up with patches.

Kindly push one of these patches if there are no changes. If not kindly suggest me a better way you think we can do this together.

Have a nice day ahead.

Thanks and regards,
Aditya.







From: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
Date: Tuesday, 21 March 2023 at 7:56 PM
To: gdb-patches@sourceware.org <gdb-patches@sourceware.org>, tom@tromey.com <tom@tromey.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>, Aditya Kamath1 <Aditya.Kamath1@ibm.com>, simon.marchi@efficios.com <simon.marchi@efficios.com>
Subject: Re: [PATCH] Modify align-c/align-c++ test case for AIX
Tom Tromey <tom@tromey.com> wrote:
>Ulrich> If the -bnogc flag is always necessary with the AIX linker, this
>Ulrich> should be handled automatically by the test instead of via site.exp.
>
>Ulrich> I'm assuming this is only needed with test files that use global
>Ulrich> variables unused in the source code, but used by the debugger.
>Ulrich> I think it would be best to explicitly add the flag to those tests
>Ulrich> where it is needed.
>
>I think there's also cases in the test suite where we've added uses of
>variables to ensure they aren't optimized away.  With a comment
>somewhere explaining why, I'm also ok with that.  It seems less
>intrusive maybe than adding ldflags to a lot of tests.

Well, that was Aditya's original proposal, I guess ...   I'd be fine
with that as well - whatever is easier.

I think doing a global run with -Wl,-bnogc would still be interesting,
simply to get a list of all the tests that would need changing.

Bye,
Ulrich

[-- Attachment #2: 0001-Modify-align-c-align-c-test-case-for-AIX_v1.patch --]
[-- Type: application/octet-stream, Size: 1004 bytes --]

From 5daed42ecd34d0019f3a2e4828a5b014ee865452 Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
Date: Fri, 10 Mar 2023 01:22:14 -0600
Subject: [PATCH] Modify align-c/align-c++ test case for AIX

On AIX, the debugger cannot access global variables before they
they are used in any function. Hence we add this small change so
that we can test the same in AIX.
---
 gdb/testsuite/gdb.base/align.exp.tcl | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gdb/testsuite/gdb.base/align.exp.tcl b/gdb/testsuite/gdb.base/align.exp.tcl
index 6a75a14d887..f18243080ce 100644
--- a/gdb/testsuite/gdb.base/align.exp.tcl
+++ b/gdb/testsuite/gdb.base/align.exp.tcl
@@ -109,6 +109,11 @@ proc prepare_test_source_file { lang } {
     # Epilogue.
     puts $outfile {
 	int main() {
+	    #ifdef _AIX
+	    /* On AIX, the debugger cannot access global variables before they
+		they are used in the functions.  */
+		a_char_x_char++;
+	    #endif
 	    return 0;
 	}
     }
-- 
2.38.3


[-- Attachment #3: 0001-Modify-align-c-align-c-test-case-for-AIX_v2.patch --]
[-- Type: application/octet-stream, Size: 1189 bytes --]

From 27aedd08296fe880bc2f2c74f9ff68713ce14245 Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
Date: Wed, 29 Mar 2023 05:18:45 -0500
Subject: [PATCH] Modify align-c/align-c++ test case for AIX

On AIX, the debugger cannot access global variables before they
they are used in any function. Hence we add this small change so
that we can test the same in AIX.
---
 gdb/testsuite/gdb.base/align.exp.tcl | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gdb/testsuite/gdb.base/align.exp.tcl b/gdb/testsuite/gdb.base/align.exp.tcl
index 6a75a14d887..18a370df72d 100644
--- a/gdb/testsuite/gdb.base/align.exp.tcl
+++ b/gdb/testsuite/gdb.base/align.exp.tcl
@@ -129,6 +129,11 @@ proc run_alignment_test { lang } {
     if { "$lang" == "c++" } {
 	lappend flags "additional_flags=-std=c++11"
     }
+    if { [istarget *-*-aix*] } {
+        # Disable linker garbage collection to avoid optimizing
+        # out global variables unused in the test source code.
+        lappend flags "ldflags=-Wl,-ua_char_x_char"
+    }
     standard_testfile $filename
     if {[prepare_for_testing "failed to prepare" "$testfile" $srcfile $flags]} {
 	return -1
-- 
2.38.3

Re: [PATCH] Modify align-c/align-c++ test case for AIX

[Pedro Alves]

On 2023-03-29 12:28 p.m., Aditya Kamath1 wrote:

>>I'm wondering whether __attribute__((used)) on the global variable makes any
>>difference.  IIRC with LLVM it prevents link-time stripping on Mach-O and PE/COFF,
>>but I don't know about XCOFF + GCC.  (It doesn't on ELF, you
>>need 'retain' for that.)
> 
> Hi Pedro. So thanks for this suggestion. I learnt something new from you. But this __attribute__((used)) did not make any difference in our environment.

Thanks for checking.

I also recalled that Clang with LTO has a similar issue, and there __attribute__((used))
tends to help.  Seems to me that it's best to tweak the code generator to just emit uses
for all variables then, instead of special casing some platform or some specific variable.

Like below.  Does this work for AIX?

From 576c9c77ff3ac297a58662471fd0a2546a1d5f42 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Wed, 29 Mar 2023 13:21:20 +0100
Subject: [PATCH] Fix gdb.base/align-*.exp and Clang + LTO and AIX GCC

Clang with LTO (clang -flto) garbage collects unused global variables,
Thus, gdb.base/align-c.exp and gdb.base/align-c++.exp fail with
hundreds of FAILs like so:

 $ make check \
    TESTS="gdb.*/align-*.exp" \
    RUNTESTFLAGS="CC_FOR_TARGET='clang -flto' CXX_FOR_TARGET='clang++ -flto'"
 ...
 FAIL: gdb.base/align-c.exp: get integer valueof "a_char"
 FAIL: gdb.base/align-c.exp: print _Alignof(char)
 FAIL: gdb.base/align-c.exp: get integer valueof "a_char_x_char"
 FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_char_x_char)
 FAIL: gdb.base/align-c.exp: get integer valueof "a_char_x_unsigned_char"
 ...

AIX GCC has the same issue, and there the easier way of adding
__attribute__((used)) to globals does not help.

So add explicit uses of all globals to the generated code.

For the C++ test, that reveals that the static variable members of the
generated structs are not defined anywhere, leading to undefined
references.  Fixed by emitting initialization for all static members.

Lastly, I noticed that CXX_FOR_TARGET was being ignored -- that's
because the align-c++.exp testcase is compiling with the C compiler
driver.  Fixed by passing "c++" as option to prepare_for_testing.

Change-Id: I874b717afde7b6fb1e45e526912b518a20a12716
---
 gdb/testsuite/gdb.base/align.exp.tcl | 49 ++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/align.exp.tcl b/gdb/testsuite/gdb.base/align.exp.tcl
index 6a75a14d887..550afe1c47d 100644
--- a/gdb/testsuite/gdb.base/align.exp.tcl
+++ b/gdb/testsuite/gdb.base/align.exp.tcl
@@ -94,12 +94,15 @@ proc prepare_test_source_file { lang } {
 		puts $outfile "DEF_WITH_1_STATIC ($utype, $uinner);"
 		set joined "static_${utype}_x_${uinner}"
 		puts $outfile "struct align_pair_$joined item_${joined};"
+		puts $outfile "$utype align_pair_${joined}::one = 0;"
 		puts $outfile "unsigned a_${joined}"
 		puts $outfile "  = ${align_func} (struct align_pair_${joined});"
 
 		puts $outfile "DEF_WITH_2_STATIC ($utype, $uinner);"
 		set joined "static_${utype}_x_static_${uinner}"
 		puts $outfile "struct align_pair_$joined item_${joined};"
+		puts $outfile "$utype align_pair_${joined}::one = 0;"
+		puts $outfile "$uinner align_pair_${joined}::two = 0;"
 		puts $outfile "unsigned a_${joined}"
 		puts $outfile "  = ${align_func} (struct align_pair_${joined});"
 	    }
@@ -107,12 +110,53 @@ proc prepare_test_source_file { lang } {
     }
 
     # Epilogue.
-    puts $outfile {
+    puts $outfile "
 	int main() {
-	    return 0;
+    "
+
+    # Clang with LTO garbage collects unused global variables, even at
+    # -O0.  Likewise AIX GCC.  Add uses to all global variables to
+    # prevent it.
+
+    if { $lang == "c" } {
+	puts $outfile "a_void++;"
+    }
+
+    # First, add uses for single items.
+    foreach type $typelist {
+	set utype [join [split $type] _]
+	puts $outfile "item_$utype++;"
+	if { $lang == "c" } {
+	    puts $outfile "a_$utype++;"
+	}
+    }
+
+    # Now add uses for all pairs.
+    foreach type $typelist {
+	set utype [join [split $type] _]
+	foreach inner $typelist {
+	    set uinner [join [split $inner] _]
+	    set joined "${utype}_x_${uinner}"
+	    puts $outfile "item_${joined}.one++;"
+	    puts $outfile "a_${joined}++;"
+
+	    if { $lang == "c++" } {
+		set joined "static_${utype}_x_${uinner}"
+		puts $outfile "item_${joined}.one++;"
+		puts $outfile "a_${joined}++;"
+
+		set joined "static_${utype}_x_static_${uinner}"
+		puts $outfile "item_${joined}.one++;"
+		puts $outfile "a_${joined}++;"
+	    }
 	}
     }
 
+    puts $outfile "
+	    return 0;
+	}
+    "
+
     close $outfile
 
     return $filename
@@ -127,6 +171,7 @@ proc run_alignment_test { lang } {
 
     set flags {debug}
     if { "$lang" == "c++" } {
+	lappend flags "c++"
 	lappend flags "additional_flags=-std=c++11"
     }
     standard_testfile $filename

base-commit: b863b097ee7a79b1114b9dd386864bc3c23a51b0
-- 
2.36.0

RE: [PATCH] Modify align-c/align-c++ test case for AIX

[Aditya Kamath1]

[-- Attachment #1.1: Type: text/plain, Size: 9573 bytes --]

Hi Pedro, Ulrich, Tom and community members,

Thank you for your feedback. Please find attached the patch. {See: 0001-Modify-align-c-align-c-test-case-for-AIX.patch}

>Like below.  Does this work for AIX?

Hi Pedro and Respected community members. Kindly allow me to disagree with you all for the patch diff that was pasted in the previous email reply thread.

Consider a smaller piece of the test case pasted
#include <iostream>
using namespace std;
struct align_pair_char_x_char {char one; char two;};
struct align_pair_char_x_char item_char_x_char;
char align_pair_char_x_char::one = 0;
unsigned a_char_x_char
  = alignof (struct align_pair_char_x_char);
int main() {
  printf ("%d \n", a_char_x_char);
}

So the line highlighted in bold is a problem. This line is also there in the patch diff that was pasted in the previous email reply thread.  This is not allowed. We are trying to access something we should not. In case we want to initialise any data member, we need to do it via a constructor.

In AIX, I can see 'char align_pair_char_x_unsigned_char::one' is not a static data member of 'struct align_pair_char_x_unsigned_char' error

I did not try in PowerPc Linux or another Linux machine. But from the little knowledge I have, it should not work there as well. Someone needs to check this in Linux. May be you or Ulrich or Tom can confirm this from your experience in Linux.  Let me know.

In my patch I have these lines.

    if { $lang == "c++" } {
+              puts $outfile "{ T one = 0; U two; }"
               puts -nonewline $outfile "#define DEF_WITH_1_STATIC(T,U) struct align_pair_static_ ## T ## _x_ ## U "
               puts $outfile "{ static T one; U two; }"
               puts -nonewline $outfile "#define DEF_WITH_2_STATIC(T,U) struct align_pair_static_ ## T ## _x_static_ ## U "
               puts $outfile "{ static T one; static U two; }"
     }
     if { $lang == "c" } {
+              puts $outfile "{ T one; U two; }"

If the aim is to initialise, then this will work in C++. In C we will get an error saying for doing one = 0,

1 | define DEF(T,U) struct align_pair_ ## T ## _x_ ## U { T one = 0; U two; }
      |                                                             ^

/home/aditya/latest_gdb/binutils-gdb/gdb/testsuite/outputs/gdb.base/align-c/align.c:52:1: note: in expansion of macro 'DEF'
   52 | DEF (char, char);
      | ^~~
/home/aditya/latest_gdb/binutils-gdb/gdb/testsuite/outputs/gdb.base/align-c/align.c:1:62: error: expected ':', ',', ';', '}' or '__attribute__' before '=' token
    1 | define DEF(T,U) struct align_pair_ ## T ## _x_ ## U { T one = 0; U two; }

So I made those changes for respective languages such that only in c++ we make one = 0.

The patch attached works perfectly. You might wonder how when we do not use the struct variables. In AIX atleast, if we access one of the garbage or unused variable we are able to access everything else.

Kindly consider the attached patch in this email, if it works in all other targets as well. If not let me know.

Have a nice day ahead.

Test case numbers of AIX after applying this patch : align-c = 406 and align-c++ = 1134
Test case numbers of AIX before applying this patch : align-c = 0 and align-c++ = 0

Output of the below code is 1.

#include <iostream>
using namespace std;
struct align_pair_char_x_char {char one = 0; char two;};
struct align_pair_char_x_char item_char_x_char;
unsigned a_char_x_char
  = alignof (struct align_pair_char_x_char);
int main() {
  printf ("%d \n", a_char_x_char);
}

Thanks and regards,
Aditya.


From: Pedro Alves <pedro@palves.net>
Date: Wednesday, 29 March 2023 at 7:06 PM
To: Aditya Kamath1 <Aditya.Kamath1@ibm.com>, Ulrich Weigand <Ulrich.Weigand@de.ibm.com>, gdb-patches@sourceware.org <gdb-patches@sourceware.org>, tom@tromey.com <tom@tromey.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>, simon.marchi@efficios.com <simon.marchi@efficios.com>
Subject: [EXTERNAL] Re: [PATCH] Modify align-c/align-c++ test case for AIX
On 2023-03-29 12:28 p.m., Aditya Kamath1 wrote:

>>I'm wondering whether __attribute__((used)) on the global variable makes any
>>difference.  IIRC with LLVM it prevents link-time stripping on Mach-O and PE/COFF,
>>but I don't know about XCOFF + GCC.  (It doesn't on ELF, you
>>need 'retain' for that.)
>
> Hi Pedro. So thanks for this suggestion. I learnt something new from you. But this __attribute__((used)) did not make any difference in our environment.

Thanks for checking.

I also recalled that Clang with LTO has a similar issue, and there __attribute__((used))
tends to help.  Seems to me that it's best to tweak the code generator to just emit uses
for all variables then, instead of special casing some platform or some specific variable.

Like below.  Does this work for AIX?

From 576c9c77ff3ac297a58662471fd0a2546a1d5f42 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Wed, 29 Mar 2023 13:21:20 +0100
Subject: [PATCH] Fix gdb.base/align-*.exp and Clang + LTO and AIX GCC

Clang with LTO (clang -flto) garbage collects unused global variables,
Thus, gdb.base/align-c.exp and gdb.base/align-c++.exp fail with
hundreds of FAILs like so:

 $ make check \
    TESTS="gdb.*/align-*.exp" \
    RUNTESTFLAGS="CC_FOR_TARGET='clang -flto' CXX_FOR_TARGET='clang++ -flto'"
 ...
 FAIL: gdb.base/align-c.exp: get integer valueof "a_char"
 FAIL: gdb.base/align-c.exp: print _Alignof(char)
 FAIL: gdb.base/align-c.exp: get integer valueof "a_char_x_char"
 FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_char_x_char)
 FAIL: gdb.base/align-c.exp: get integer valueof "a_char_x_unsigned_char"
 ...

AIX GCC has the same issue, and there the easier way of adding
__attribute__((used)) to globals does not help.

So add explicit uses of all globals to the generated code.

For the C++ test, that reveals that the static variable members of the
generated structs are not defined anywhere, leading to undefined
references.  Fixed by emitting initialization for all static members.

Lastly, I noticed that CXX_FOR_TARGET was being ignored -- that's
because the align-c++.exp testcase is compiling with the C compiler
driver.  Fixed by passing "c++" as option to prepare_for_testing.

Change-Id: I874b717afde7b6fb1e45e526912b518a20a12716
---
 gdb/testsuite/gdb.base/align.exp.tcl | 49 ++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/align.exp.tcl b/gdb/testsuite/gdb.base/align.exp.tcl
index 6a75a14d887..550afe1c47d 100644
--- a/gdb/testsuite/gdb.base/align.exp.tcl
+++ b/gdb/testsuite/gdb.base/align.exp.tcl
@@ -94,12 +94,15 @@ proc prepare_test_source_file { lang } {
                 puts $outfile "DEF_WITH_1_STATIC ($utype, $uinner);"
                 set joined "static_${utype}_x_${uinner}"
                 puts $outfile "struct align_pair_$joined item_${joined};"
+               puts $outfile "$utype align_pair_${joined}::one = 0;"
                 puts $outfile "unsigned a_${joined}"
                 puts $outfile "  = ${align_func} (struct align_pair_${joined});"

                 puts $outfile "DEF_WITH_2_STATIC ($utype, $uinner);"
                 set joined "static_${utype}_x_static_${uinner}"
                 puts $outfile "struct align_pair_$joined item_${joined};"
+               puts $outfile "$utype align_pair_${joined}::one = 0;"
+               puts $outfile "$uinner align_pair_${joined}::two = 0;"
                 puts $outfile "unsigned a_${joined}"
                 puts $outfile "  = ${align_func} (struct align_pair_${joined});"
             }
@@ -107,12 +110,53 @@ proc prepare_test_source_file { lang } {
     }

     # Epilogue.
-    puts $outfile {
+    puts $outfile "
         int main() {
-           return 0;
+    "
+
+    # Clang with LTO garbage collects unused global variables, even at
+    # -O0.  Likewise AIX GCC.  Add uses to all global variables to
+    # prevent it.
+
+    if { $lang == "c" } {
+       puts $outfile "a_void++;"
+    }
+
+    # First, add uses for single items.
+    foreach type $typelist {
+       set utype [join [split $type] _]
+       puts $outfile "item_$utype++;"
+       if { $lang == "c" } {
+           puts $outfile "a_$utype++;"
+       }
+    }
+
+    # Now add uses for all pairs.
+    foreach type $typelist {
+       set utype [join [split $type] _]
+       foreach inner $typelist {
+           set uinner [join [split $inner] _]
+           set joined "${utype}_x_${uinner}"
+           puts $outfile "item_${joined}.one++;"
+           puts $outfile "a_${joined}++;"
+
+           if { $lang == "c++" } {
+               set joined "static_${utype}_x_${uinner}"
+               puts $outfile "item_${joined}.one++;"
+               puts $outfile "a_${joined}++;"
+
+               set joined "static_${utype}_x_static_${uinner}"
+               puts $outfile "item_${joined}.one++;"
+               puts $outfile "a_${joined}++;"
+           }
         }
     }

+    puts $outfile "
+           return 0;
+       }
+    "
+
     close $outfile

     return $filename
@@ -127,6 +171,7 @@ proc run_alignment_test { lang } {

     set flags {debug}
     if { "$lang" == "c++" } {
+       lappend flags "c++"
         lappend flags "additional_flags=-std=c++11"
     }
     standard_testfile $filename

base-commit: b863b097ee7a79b1114b9dd386864bc3c23a51b0
--
2.36.0

[-- Attachment #2: 0001-Modify-align-c-align-c-test-case-for-AIX.patch --]
[-- Type: application/octet-stream, Size: 2834 bytes --]

From 61e3591ecfa752ffa25278230eee2a8c18b83f86 Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
Date: Fri, 31 Mar 2023 07:08:22 -0500
Subject: [PATCH] Modify align-c/align-c++ test case for AIX

On AIX, the debugger cannot access global variables before they
they are used in any function. Hence we add this small change so
that we can test the same in AIX.
---
 gdb/testsuite/gdb.base/align.exp.tcl | 46 +++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.base/align.exp.tcl b/gdb/testsuite/gdb.base/align.exp.tcl
index 6a75a14d887..801ee1648c9 100644
--- a/gdb/testsuite/gdb.base/align.exp.tcl
+++ b/gdb/testsuite/gdb.base/align.exp.tcl
@@ -55,14 +55,15 @@ proc prepare_test_source_file { lang } {
 
     # Prologue.
     puts -nonewline $outfile "#define DEF(T,U) struct align_pair_ ## T ## _x_ ## U "
-    puts $outfile "{ T one; U two; }"
     if { $lang == "c++" } {
+	puts $outfile "{ T one = 0; U two; }"
 	puts -nonewline $outfile "#define DEF_WITH_1_STATIC(T,U) struct align_pair_static_ ## T ## _x_ ## U "
 	puts $outfile "{ static T one; U two; }"
 	puts -nonewline $outfile "#define DEF_WITH_2_STATIC(T,U) struct align_pair_static_ ## T ## _x_static_ ## U "
 	puts $outfile "{ static T one; static U two; }"
     }
     if { $lang == "c" } {
+	puts $outfile "{ T one; U two; }"
 	puts $outfile "unsigned a_void = ${align_func} (void);"
     }
 
@@ -107,12 +108,48 @@ proc prepare_test_source_file { lang } {
     }
 
     # Epilogue.
-    puts $outfile {
+    puts $outfile "
 	int main() {
-	    return 0;
+	"
+	# Clang with LTO garbage collects unused global variables, even at
+	# -O0.  Likewise AIX GCC.  Add uses to all global variables to
+	# prevent it.
+	if { $lang == "c" } {
+	  puts $outfile "a_void++;"
+	}
+
+	# First, add uses for single items.
+	foreach type $typelist {
+	  set utype [join [split $type] _]
+	  puts $outfile "item_$utype++;"
+	  if { $lang == "c" } {
+	    puts $outfile "a_$utype++;"
+	  }
+	}
+
+	# Now add uses for all pairs.
+	foreach type $typelist {
+	  set utype [join [split $type] _]
+	  foreach inner $typelist {
+	    set uinner [join [split $inner] _]
+	    set joined "${utype}_x_${uinner}"
+	    puts $outfile "a_${joined}++;"
+	  }
 	}
-    }
 
+	if { $lang == "c++" } {
+	  set joined "static_${utype}_x_${uinner}"
+	  puts $outfile "a_${joined}++;"
+
+	  set joined "static_${utype}_x_static_${uinner}"
+	  puts $outfile "a_${joined}++;"
+	}
+    
+    puts $outfile "
+      return 0;
+    } 
+    "
+    
     close $outfile
 
     return $filename
@@ -127,6 +164,7 @@ proc run_alignment_test { lang } {
 
     set flags {debug}
     if { "$lang" == "c++" } {
+	lappend flags "c++"
 	lappend flags "additional_flags=-std=c++11"
     }
     standard_testfile $filename
-- 
2.38.3

RE: [PATCH] Modify align-c/align-c++ test case for AIX

[Aditya Kamath1]

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

Hi all,

Kindly update me on my patch in the previous email…

Thanks and regards,
Aditya.

From: Gdb-patches <gdb-patches-bounces+aditya.kamath1=ibm.com@sourceware.org> on behalf of Aditya Kamath1 via Gdb-patches <gdb-patches@sourceware.org>
Date: Friday, 31 March 2023 at 5:59 PM
To: Pedro Alves <pedro@palves.net>, Ulrich Weigand <Ulrich.Weigand@de.ibm.com>, gdb-patches@sourceware.org <gdb-patches@sourceware.org>, tom@tromey.com <tom@tromey.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>, simon.marchi@efficios.com <simon.marchi@efficios.com>
Subject: [EXTERNAL] RE: [PATCH] Modify align-c/align-c++ test case for AIX
Hi Pedro, Ulrich, Tom and community members,

Thank you for your feedback. Please find attached the patch. {See: 0001-Modify-align-c-align-c-test-case-for-AIX.patch}

>Like below.  Does this work for AIX?

Hi Pedro and Respected community members. Kindly allow me to disagree with you all for the patch diff that was pasted in the previous email reply thread.

Consider a smaller piece of the test case pasted
#include <iostream>
using namespace std;
struct align_pair_char_x_char {char one; char two;};
struct align_pair_char_x_char item_char_x_char;
char align_pair_char_x_char::one = 0;
unsigned a_char_x_char
  = alignof (struct align_pair_char_x_char);
int main() {
  printf ("%d \n", a_char_x_char);
}

So the line highlighted in bold is a problem. This line is also there in the patch diff that was pasted in the previous email reply thread.  This is not allowed. We are trying to access something we should not. In case we want to initialise any data member, we need to do it via a constructor.

In AIX, I can see 'char align_pair_char_x_unsigned_char::one' is not a static data member of 'struct align_pair_char_x_unsigned_char' error

I did not try in PowerPc Linux or another Linux machine. But from the little knowledge I have, it should not work there as well. Someone needs to check this in Linux. May be you or Ulrich or Tom can confirm this from your experience in Linux.  Let me know.

In my patch I have these lines.

    if { $lang == "c++" } {
+              puts $outfile "{ T one = 0; U two; }"
               puts -nonewline $outfile "#define DEF_WITH_1_STATIC(T,U) struct align_pair_static_ ## T ## _x_ ## U "
               puts $outfile "{ static T one; U two; }"
               puts -nonewline $outfile "#define DEF_WITH_2_STATIC(T,U) struct align_pair_static_ ## T ## _x_static_ ## U "
               puts $outfile "{ static T one; static U two; }"
     }
     if { $lang == "c" } {
+              puts $outfile "{ T one; U two; }"

If the aim is to initialise, then this will work in C++. In C we will get an error saying for doing one = 0,

1 | define DEF(T,U) struct align_pair_ ## T ## _x_ ## U { T one = 0; U two; }
      |                                                             ^

/home/aditya/latest_gdb/binutils-gdb/gdb/testsuite/outputs/gdb.base/align-c/align.c:52:1: note: in expansion of macro 'DEF'
   52 | DEF (char, char);
      | ^~~
/home/aditya/latest_gdb/binutils-gdb/gdb/testsuite/outputs/gdb.base/align-c/align.c:1:62: error: expected ':', ',', ';', '}' or '__attribute__' before '=' token
    1 | define DEF(T,U) struct align_pair_ ## T ## _x_ ## U { T one = 0; U two; }

So I made those changes for respective languages such that only in c++ we make one = 0.

The patch attached works perfectly. You might wonder how when we do not use the struct variables. In AIX atleast, if we access one of the garbage or unused variable we are able to access everything else.

Kindly consider the attached patch in this email, if it works in all other targets as well. If not let me know.

Have a nice day ahead.

Test case numbers of AIX after applying this patch : align-c = 406 and align-c++ = 1134
Test case numbers of AIX before applying this patch : align-c = 0 and align-c++ = 0

Output of the below code is 1.

#include <iostream>
using namespace std;
struct align_pair_char_x_char {char one = 0; char two;};
struct align_pair_char_x_char item_char_x_char;
unsigned a_char_x_char
  = alignof (struct align_pair_char_x_char);
int main() {
  printf ("%d \n", a_char_x_char);
}

Thanks and regards,
Aditya.


From: Pedro Alves <pedro@palves.net>
Date: Wednesday, 29 March 2023 at 7:06 PM
To: Aditya Kamath1 <Aditya.Kamath1@ibm.com>, Ulrich Weigand <Ulrich.Weigand@de.ibm.com>, gdb-patches@sourceware.org <gdb-patches@sourceware.org>, tom@tromey.com <tom@tromey.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>, simon.marchi@efficios.com <simon.marchi@efficios.com>
Subject: [EXTERNAL] Re: [PATCH] Modify align-c/align-c++ test case for AIX
On 2023-03-29 12:28 p.m., Aditya Kamath1 wrote:

>>I'm wondering whether __attribute__((used)) on the global variable makes any
>>difference.  IIRC with LLVM it prevents link-time stripping on Mach-O and PE/COFF,
>>but I don't know about XCOFF + GCC.  (It doesn't on ELF, you
>>need 'retain' for that.)
>
> Hi Pedro. So thanks for this suggestion. I learnt something new from you. But this __attribute__((used)) did not make any difference in our environment.

Thanks for checking.

I also recalled that Clang with LTO has a similar issue, and there __attribute__((used))
tends to help.  Seems to me that it's best to tweak the code generator to just emit uses
for all variables then, instead of special casing some platform or some specific variable.

Like below.  Does this work for AIX?

From 576c9c77ff3ac297a58662471fd0a2546a1d5f42 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Wed, 29 Mar 2023 13:21:20 +0100
Subject: [PATCH] Fix gdb.base/align-*.exp and Clang + LTO and AIX GCC

Clang with LTO (clang -flto) garbage collects unused global variables,
Thus, gdb.base/align-c.exp and gdb.base/align-c++.exp fail with
hundreds of FAILs like so:

 $ make check \
    TESTS="gdb.*/align-*.exp" \
    RUNTESTFLAGS="CC_FOR_TARGET='clang -flto' CXX_FOR_TARGET='clang++ -flto'"
 ...
 FAIL: gdb.base/align-c.exp: get integer valueof "a_char"
 FAIL: gdb.base/align-c.exp: print _Alignof(char)
 FAIL: gdb.base/align-c.exp: get integer valueof "a_char_x_char"
 FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_char_x_char)
 FAIL: gdb.base/align-c.exp: get integer valueof "a_char_x_unsigned_char"
 ...

AIX GCC has the same issue, and there the easier way of adding
__attribute__((used)) to globals does not help.

So add explicit uses of all globals to the generated code.

For the C++ test, that reveals that the static variable members of the
generated structs are not defined anywhere, leading to undefined
references.  Fixed by emitting initialization for all static members.

Lastly, I noticed that CXX_FOR_TARGET was being ignored -- that's
because the align-c++.exp testcase is compiling with the C compiler
driver.  Fixed by passing "c++" as option to prepare_for_testing.

Change-Id: I874b717afde7b6fb1e45e526912b518a20a12716
---
 gdb/testsuite/gdb.base/align.exp.tcl | 49 ++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/align.exp.tcl b/gdb/testsuite/gdb.base/align.exp.tcl
index 6a75a14d887..550afe1c47d 100644
--- a/gdb/testsuite/gdb.base/align.exp.tcl
+++ b/gdb/testsuite/gdb.base/align.exp.tcl
@@ -94,12 +94,15 @@ proc prepare_test_source_file { lang } {
                 puts $outfile "DEF_WITH_1_STATIC ($utype, $uinner);"
                 set joined "static_${utype}_x_${uinner}"
                 puts $outfile "struct align_pair_$joined item_${joined};"
+               puts $outfile "$utype align_pair_${joined}::one = 0;"
                 puts $outfile "unsigned a_${joined}"
                 puts $outfile "  = ${align_func} (struct align_pair_${joined});"

                 puts $outfile "DEF_WITH_2_STATIC ($utype, $uinner);"
                 set joined "static_${utype}_x_static_${uinner}"
                 puts $outfile "struct align_pair_$joined item_${joined};"
+               puts $outfile "$utype align_pair_${joined}::one = 0;"
+               puts $outfile "$uinner align_pair_${joined}::two = 0;"
                 puts $outfile "unsigned a_${joined}"
                 puts $outfile "  = ${align_func} (struct align_pair_${joined});"
             }
@@ -107,12 +110,53 @@ proc prepare_test_source_file { lang } {
     }

     # Epilogue.
-    puts $outfile {
+    puts $outfile "
         int main() {
-           return 0;
+    "
+
+    # Clang with LTO garbage collects unused global variables, even at
+    # -O0.  Likewise AIX GCC.  Add uses to all global variables to
+    # prevent it.
+
+    if { $lang == "c" } {
+       puts $outfile "a_void++;"
+    }
+
+    # First, add uses for single items.
+    foreach type $typelist {
+       set utype [join [split $type] _]
+       puts $outfile "item_$utype++;"
+       if { $lang == "c" } {
+           puts $outfile "a_$utype++;"
+       }
+    }
+
+    # Now add uses for all pairs.
+    foreach type $typelist {
+       set utype [join [split $type] _]
+       foreach inner $typelist {
+           set uinner [join [split $inner] _]
+           set joined "${utype}_x_${uinner}"
+           puts $outfile "item_${joined}.one++;"
+           puts $outfile "a_${joined}++;"
+
+           if { $lang == "c++" } {
+               set joined "static_${utype}_x_${uinner}"
+               puts $outfile "item_${joined}.one++;"
+               puts $outfile "a_${joined}++;"
+
+               set joined "static_${utype}_x_static_${uinner}"
+               puts $outfile "item_${joined}.one++;"
+               puts $outfile "a_${joined}++;"
+           }
         }
     }

+    puts $outfile "
+           return 0;
+       }
+    "
+
     close $outfile

     return $filename
@@ -127,6 +171,7 @@ proc run_alignment_test { lang } {

     set flags {debug}
     if { "$lang" == "c++" } {
+       lappend flags "c++"
         lappend flags "additional_flags=-std=c++11"
     }
     standard_testfile $filename

base-commit: b863b097ee7a79b1114b9dd386864bc3c23a51b0
--
2.36.0

Re: [PATCH] Modify align-c/align-c++ test case for AIX

[Pedro Alves]

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

On 2023-03-31 1:29 p.m., Aditya Kamath1 wrote:
> Hi Pedro, Ulrich, Tom and community members,
> 
> Thank you for your feedback. Please find attached the patch. {See: 0001-Modify-align-c-align-c-test-case-for-AIX.patch}
> 
>>Like below.  Does this work for AIX?
> 
> Hi Pedro and Respected community members. Kindly allow me to disagree with you all for the patch diff that was pasted in the previous email reply thread.
> 
> Consider a smaller piece of the test case pasted
> #include <iostream>
> 
> using namespace std;
> 
> struct align_pair_char_x_char {char one; char two;};
> 
> struct align_pair_char_x_char item_char_x_char;
> 
> *char align_pair_char_x_char::one = 0;*
> 
> unsigned a_char_x_char
> 
>   = alignof (struct align_pair_char_x_char);
> 
> int main() {
> 
>   printf ("%d \n", a_char_x_char);
> 
> }  
> 
> So the line highlighted in bold is a problem. This line is also there in the patch diff that was pasted in the previous email reply thread.  This is not allowed. We are trying to access something we should not. In case we want to initialise any data member, we need to do it via a constructor.


I'm super confused.  I don't get a line like the one you highlighted in the generated code at all.

> 
> In AIX, I can see 'char align_pair_char_x_unsigned_char::one' is not a static data member of 'struct align_pair_char_x_unsigned_char' error

How did you get that line in the generated code after my patch?

Note the patch has:

> diff --git a/gdb/testsuite/gdb.base/align.exp.tcl b/gdb/testsuite/gdb.base/align.exp.tcl
> index 6a75a14d887..550afe1c47d 100644
> --- a/gdb/testsuite/gdb.base/align.exp.tcl
> +++ b/gdb/testsuite/gdb.base/align.exp.tcl
> @@ -94,12 +94,15 @@ proc prepare_test_source_file { lang } {
>                  puts $outfile "DEF_WITH_1_STATIC ($utype, $uinner);"
>                  set joined "static_${utype}_x_${uinner}"
>                  puts $outfile "struct align_pair_$joined item_${joined};"
> +               puts $outfile "$utype align_pair_${joined}::one = 0;"
>                  puts $outfile "unsigned a_${joined}"
>                  puts $outfile "  = ${align_func} (struct align_pair_${joined});"
>  
>                  puts $outfile "DEF_WITH_2_STATIC ($utype, $uinner);"
>                  set joined "static_${utype}_x_static_${uinner}"
>                  puts $outfile "struct align_pair_$joined item_${joined};"
> +               puts $outfile "$utype align_pair_${joined}::one = 0;"
> +               puts $outfile "$uinner align_pair_${joined}::two = 0;"
>                  puts $outfile "unsigned a_${joined}"
>                  puts $outfile "  = ${align_func} (struct align_pair_${joined});"
>              }

DEF_WITH_1_STATIC and DEF_WITH_2_STATIC look like this:

 #define DEF_WITH_1_STATIC(T,U) struct align_pair_static_ ## T ## _x_ ## U { static T one; U two; }
 #define DEF_WITH_2_STATIC(T,U) struct align_pair_static_ ## T ## _x_static_ ## U { static T one; static U two; }

The first has one static field, the second has two static fields.  The lines I added in the
patch hunk above are meant to initialize those static fields.  That's why I have:

Initialize one static field here:

                  puts $outfile "DEF_WITH_1_STATIC ($utype, $uinner);"
                  ...
                  puts $outfile "$utype align_pair_${joined}::one = 0;"

Initialize two static fields here:

                  puts $outfile "DEF_WITH_2_STATIC ($utype, $uinner);"
                  ...
                  puts $outfile "$utype align_pair_${joined}::one = 0;"
                  puts $outfile "$uinner align_pair_${joined}::two = 0;"

Both of these commands came out empty for me, meaning the "::one" and "::two"
initializations are only done on lines with types that have "static" in their name:

 $ grep "::one" testsuite/outputs/gdb.base/align-c++/align.cpp  | grep -v static
 $ grep "::two" testsuite/outputs/gdb.base/align-c++/align.cpp  | grep -v static


Diffing the align.cpp file generated before my patch against a align.cpp file 
generated after my patch, I get a lot of changes that looks like this:

--- align.cpp   2023-04-05 17:20:54.002858720 +0100
+++ align-pedro.cpp     2023-04-05 17:20:33.228766715 +0100
@@ -28,10 +28,13 @@ unsigned a_char_x_char
   = alignof (struct align_pair_char_x_char);
 DEF_WITH_1_STATIC (char, char);
 struct align_pair_static_char_x_char item_static_char_x_char;
+char align_pair_static_char_x_char::one = 0;
 unsigned a_static_char_x_char
   = alignof (struct align_pair_static_char_x_char);
 DEF_WITH_2_STATIC (char, char);
 struct align_pair_static_char_x_static_char item_static_char_x_static_char;
+char align_pair_static_char_x_static_char::one = 0;
+char align_pair_static_char_x_static_char::two = 0;
...

Again, the "::one" and "::one" lines only appear on types that have "static"
in their name.

> 
> 
> I did not try in PowerPc Linux or another Linux machine. But from the little knowledge I have, it should not work there as well. Someone needs to check this in Linux. May be you or Ulrich or Tom can confirm this from your experience in Linux.  Let me know.
> 

I had tested my patch on Linux before sending it.  It works here just fine, like so:

 $ make check TESTS="gdb.*/align*.exp"
 Running /home/pedro/rocm/gdb/build-master/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/align-c++.exp ...
 Running /home/pedro/rocm/gdb/build-master/gdb/testsuite/../../../src/gdb/testsuite/gdb.cp/align.exp ...
 Running /home/pedro/rocm/gdb/build-master/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/align-c.exp ...
 
                 === gdb Summary ===
 
 # of expected passes            3450


Your patch doesn't fix the issue I pointed at in the commit log, this one:

    For the C++ test, that reveals that the static variable members of the
    generated structs are not defined anywhere, leading to undefined
    references.  Fixed by emitting initialization for all static members.

I.e., if I test with:

 $ make check TESTS="gdb.*/align-*.exp" RUNTESTFLAGS="CC_FOR_TARGET='clang -flto' CXX_FOR_TARGET='clang++ -flto'"

then I see hundreds of failures like these:

 print /d a_static_char_x_char
 Missing ELF symbol "a_static_char_x_char".
 (gdb) FAIL: gdb.base/align-c++.exp: get integer valueof "a_static_char_x_char"
 ...


Going back to this:

> So the line highlighted in bold is a problem. This line is also there in the patch diff that was pasted in the previous email reply thread.  This is not allowed. We are trying to access something we should not. In case we want to initialise any data member, we need to do it via a constructor.

I'm attaching the attach.cpp file that the testsuite generates for me.

[-- Attachment #2: align.cpp.gz --]
[-- Type: application/gzip, Size: 15008 bytes --]

RE: [PATCH] Modify align-c/align-c++ test case for AIX

[Aditya Kamath1]

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

Hi Pedro, Ulrich, Tom and community,

I actually made a mistake while I was experimenting. I copied this line in two places resulting in this hunk by mistake which was not required. That is why I saw the error of initialising a non-static member. I apologise for the same.

 @@ -87,6 +87,7 @@
            puts $outfile "DEF ($utype, $uinner);"
            set joined "${utype}_x_${uinner}"
            puts $outfile "struct align_pair_$joined item_${joined};"
+           puts $outfile "$utype align_pair_${joined}::one = 0;"
            puts $outfile "unsigned a_${joined}"
            puts $outfile "  = ${align_func} (struct align_pair_${joined});"

So AIX numbers are here:-
gdb.base/align-c++ = Passes = 1134
gdb.base/align-c = Passes = 406
gdb.base/align = Passes = 1802

I am okay with the patch with change ID = I874b717afde7b6fb1e45e526912b518a20a12716 pasted to me on the 29th of March. It is the same in AIX as well. Kindly commit the same.


Only one more problem Pedro, Tom and Ulrich..
Consider the code below..

#include <iostream>
using namespace std;
#define DEF(T,U) struct align_pair_ ## T ## _x_ ## U { T one; U two; }
DEF (char, double);
struct align_pair_char_x_double item_char_x_double;
int main (){
    int x = alignof (struct align_pair_char_x_double);
    printf ("%d \n", x);
    return 0;
}

The output of this in linux is 8. But in AIX is 4. Though the size is 8 for double, alignment is 4.

bash-5.1$ ~/gdb_tests/align
4

So we have failures in all test cases involving double data type in AIX.

Using /home/aditya/latest_gdb/binutils-gdb/gdb/testsuite/config/unix.exp as tool-and-target-specific interface file.
Running /home/aditya/latest_gdb/binutils-gdb/gdb/testsuite/gdb.base/align-c.exp ...
FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_char_x_double)
FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_char_x_long_double)
FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_unsigned_char_x_double)
FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_unsigned_char_x_long_double)
FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_short_x_double)
FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_short_x_long_double)
FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_unsigned_short_x_double)
FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_unsigned_short_x_long_double)
FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_int_x_double)
FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_int_x_long_double)
FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_unsigned_int_x_double)
FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_unsigned_int_x_long_double)
FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_float_x_double)
FAIL: gdb.base/align-c.exp: print _Alignof(struct align_pair_float_x_long_double)
FAIL: gdb.base/align-c.exp: print _Alignof(double)
FAIL: gdb.base/align-c.exp: print _Alignof(long double)

                === gdb Summary ===

# of expected passes            406
# of unexpected failures        16
/home/aditya/latest_gdb/binutils-gdb/gdb/gdb version  14.0.50.20230327-git -nw -nx -iex "set height 0" -iex "set width 0" -data-directory /home/aditya/latest_gdb/binutils-gdb/gdb/data-directory


Number of failures:-
gdb.base/align-c++ = 42
gdb.base/align-c = 16
gdb.cp/align = 50

Can we discuss this in the same thread or you want me to handle it in a different thread email??Let me know.


Have a nice day ahead.

Thanks and regards,
Aditya.






From: Pedro Alves <pedro@palves.net>
Date: Wednesday, 5 April 2023 at 10:03 PM
To: Aditya Kamath1 <Aditya.Kamath1@ibm.com>, Ulrich Weigand <Ulrich.Weigand@de.ibm.com>, gdb-patches@sourceware.org <gdb-patches@sourceware.org>, tom@tromey.com <tom@tromey.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>, simon.marchi@efficios.com <simon.marchi@efficios.com>
Subject: [EXTERNAL] Re: [PATCH] Modify align-c/align-c++ test case for AIX
On 2023-03-31 1:29 p.m., Aditya Kamath1 wrote:
> Hi Pedro, Ulrich, Tom and community members,
>
> Thank you for your feedback. Please find attached the patch. {See: 0001-Modify-align-c-align-c-test-case-for-AIX.patch}
>
>>Like below.  Does this work for AIX?
>
> Hi Pedro and Respected community members. Kindly allow me to disagree with you all for the patch diff that was pasted in the previous email reply thread.
>
> Consider a smaller piece of the test case pasted
> #include <iostream>
>
> using namespace std;
>
> struct align_pair_char_x_char {char one; char two;};
>
> struct align_pair_char_x_char item_char_x_char;
>
> *char align_pair_char_x_char::one = 0;*
>
> unsigned a_char_x_char
>
>   = alignof (struct align_pair_char_x_char);
>
> int main() {
>
>   printf ("%d \n", a_char_x_char);
>
> }
>
> So the line highlighted in bold is a problem. This line is also there in the patch diff that was pasted in the previous email reply thread.  This is not allowed. We are trying to access something we should not. In case we want to initialise any data member, we need to do it via a constructor.


I'm super confused.  I don't get a line like the one you highlighted in the generated code at all.

>
> In AIX, I can see 'char align_pair_char_x_unsigned_char::one' is not a static data member of 'struct align_pair_char_x_unsigned_char' error

How did you get that line in the generated code after my patch?

Note the patch has:

> diff --git a/gdb/testsuite/gdb.base/align.exp.tcl b/gdb/testsuite/gdb.base/align.exp.tcl
> index 6a75a14d887..550afe1c47d 100644
> --- a/gdb/testsuite/gdb.base/align.exp.tcl
> +++ b/gdb/testsuite/gdb.base/align.exp.tcl
> @@ -94,12 +94,15 @@ proc prepare_test_source_file { lang } {
>                  puts $outfile "DEF_WITH_1_STATIC ($utype, $uinner);"
>                  set joined "static_${utype}_x_${uinner}"
>                  puts $outfile "struct align_pair_$joined item_${joined};"
> +               puts $outfile "$utype align_pair_${joined}::one = 0;"
>                  puts $outfile "unsigned a_${joined}"
>                  puts $outfile "  = ${align_func} (struct align_pair_${joined});"
>
>                  puts $outfile "DEF_WITH_2_STATIC ($utype, $uinner);"
>                  set joined "static_${utype}_x_static_${uinner}"
>                  puts $outfile "struct align_pair_$joined item_${joined};"
> +               puts $outfile "$utype align_pair_${joined}::one = 0;"
> +               puts $outfile "$uinner align_pair_${joined}::two = 0;"
>                  puts $outfile "unsigned a_${joined}"
>                  puts $outfile "  = ${align_func} (struct align_pair_${joined});"
>              }

DEF_WITH_1_STATIC and DEF_WITH_2_STATIC look like this:

 #define DEF_WITH_1_STATIC(T,U) struct align_pair_static_ ## T ## _x_ ## U { static T one; U two; }
 #define DEF_WITH_2_STATIC(T,U) struct align_pair_static_ ## T ## _x_static_ ## U { static T one; static U two; }

The first has one static field, the second has two static fields.  The lines I added in the
patch hunk above are meant to initialize those static fields.  That's why I have:

Initialize one static field here:

                  puts $outfile "DEF_WITH_1_STATIC ($utype, $uinner);"
                  ...
                  puts $outfile "$utype align_pair_${joined}::one = 0;"

Initialize two static fields here:

                  puts $outfile "DEF_WITH_2_STATIC ($utype, $uinner);"
                  ...
                  puts $outfile "$utype align_pair_${joined}::one = 0;"
                  puts $outfile "$uinner align_pair_${joined}::two = 0;"

Both of these commands came out empty for me, meaning the "::one" and "::two"
initializations are only done on lines with types that have "static" in their name:

 $ grep "::one" testsuite/outputs/gdb.base/align-c++/align.cpp  | grep -v static
 $ grep "::two" testsuite/outputs/gdb.base/align-c++/align.cpp  | grep -v static


Diffing the align.cpp file generated before my patch against a align.cpp file
generated after my patch, I get a lot of changes that looks like this:

--- align.cpp   2023-04-05 17:20:54.002858720 +0100
+++ align-pedro.cpp     2023-04-05 17:20:33.228766715 +0100
@@ -28,10 +28,13 @@ unsigned a_char_x_char
   = alignof (struct align_pair_char_x_char);
 DEF_WITH_1_STATIC (char, char);
 struct align_pair_static_char_x_char item_static_char_x_char;
+char align_pair_static_char_x_char::one = 0;
 unsigned a_static_char_x_char
   = alignof (struct align_pair_static_char_x_char);
 DEF_WITH_2_STATIC (char, char);
 struct align_pair_static_char_x_static_char item_static_char_x_static_char;
+char align_pair_static_char_x_static_char::one = 0;
+char align_pair_static_char_x_static_char::two = 0;
...

Again, the "::one" and "::one" lines only appear on types that have "static"
in their name.

>
>
> I did not try in PowerPc Linux or another Linux machine. But from the little knowledge I have, it should not work there as well. Someone needs to check this in Linux. May be you or Ulrich or Tom can confirm this from your experience in Linux.  Let me know.
>

I had tested my patch on Linux before sending it.  It works here just fine, like so:

 $ make check TESTS="gdb.*/align*.exp"
 Running /home/pedro/rocm/gdb/build-master/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/align-c++.exp ...
 Running /home/pedro/rocm/gdb/build-master/gdb/testsuite/../../../src/gdb/testsuite/gdb.cp/align.exp ...
 Running /home/pedro/rocm/gdb/build-master/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/align-c.exp ...

                 === gdb Summary ===

 # of expected passes            3450


Your patch doesn't fix the issue I pointed at in the commit log, this one:

    For the C++ test, that reveals that the static variable members of the
    generated structs are not defined anywhere, leading to undefined
    references.  Fixed by emitting initialization for all static members.

I.e., if I test with:

 $ make check TESTS="gdb.*/align-*.exp" RUNTESTFLAGS="CC_FOR_TARGET='clang -flto' CXX_FOR_TARGET='clang++ -flto'"

then I see hundreds of failures like these:

 print /d a_static_char_x_char
 Missing ELF symbol "a_static_char_x_char".
 (gdb) FAIL: gdb.base/align-c++.exp: get integer valueof "a_static_char_x_char"
 ...


Going back to this:

> So the line highlighted in bold is a problem. This line is also there in the patch diff that was pasted in the previous email reply thread.  This is not allowed. We are trying to access something we should not. In case we want to initialise any data member, we need to do it via a constructor.

I'm attaching the attach.cpp file that the testsuite generates for me.

Re: [PATCH] Modify align-c/align-c++ test case for AIX

[Pedro Alves]

Às 14:15 de 06/04/2023, Aditya Kamath1 escreveu:
> Hi Pedro, Ulrich, Tom and community,
> 
> I actually made a mistake while I was experimenting. I copied this line 
> in two places resulting in this hunk by mistake which was not required. 
> That is why I saw the error of initialising a non-static member. I 
> apologise for the same.

Ah, OK.  Makes sense now.  No worries.


> I am okay with the patch with change ID = 
> I874b717afde7b6fb1e45e526912b518a20a12716 pasted to me on the 29^th of 
> March. It is the same in AIX as well. Kindly commit the same.

Alright, merged.

> 
> 
> Only one more problem Pedro, Tom and Ulrich..
> Consider the code below..
> 
> #include <iostream>
> 
> using namespace std;
> 
> #define DEF(T,U) struct align_pair_ ## T ## _x_ ## U { T one; U two; }
> 
> DEF (char, double);
> 
> struct align_pair_char_x_double item_char_x_double;
> 
> int main (){
> 
>      int x = alignof (struct align_pair_char_x_double);
> 
>      printf ("%d \n", x);
> 
>      return 0;
> 
> }
> 
> The output of this in linux is 8. But in AIX is 4. Though the size is 8 
> for double, alignment is 4.
> 
> bash-5.1$ ~/gdb_tests/align
> 
> 4

That it's different between Linux and AIX does not by itself indicate
a problem.

> 
> So we have failures in all test cases involving double data type in AIX.

... because GDB's alignof operator returns 8?

It sounds like what the compiler says is the type's alignment differs
from GDB's built-in knowledge of the type's alignment.  I.e., GDB
alignof implementation returns the wrong alignment.  This is exactly the
sort of bug that the testcase is meant to catch.

Grepping around, it doesn't seem like AIX installs a gdbarch_type_align
hook, so the fix may to install one, and make it handle the double type
specially.

Pedro Alves

Re: [PATCH] Modify align-c/align-c++ test case for AIX

[Aditya Kamath1]

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

Hi,

I sent this patch twice by mistake. Kindly ignore this email. I apologise for the same.

From: Gdb-patches <gdb-patches-bounces+aditya.kamath1=ibm.com@sourceware.org> on behalf of Aditya Kamath1 via Gdb-patches <gdb-patches@sourceware.org>
Date: Friday, 10 March 2023 at 2:26 PM
To: Aditya Kamath1 via Gdb-patches <gdb-patches@sourceware.org>, Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
Subject: [EXTERNAL] [PATCH] Modify align-c/align-c++ test case for AIX
Hi all,

Please find attached the patch. {See:- 0001-Modify-align-c-align-c-test-case-for-AIX.patch}

In AIX, we have observed around 1500 failures in align-c/align-c++ testcase. The reason being this testcase shows the alignment outputs of global variables that are unused in the main (). In AIX we need to use atleast one global variable to access the same. This patch is a fix to the same. This issue is like this link pasted in the next line which we fixed before. https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60204874f5a987b164f7f194d3f96729847fe329

Kindly let me know if this is okay.

Test case command:- gmake RUNTESTFLAGS="gdb.base/align-c.exp" check
                                       gmake RUNTESTFLAGS="gdb.base/align-c++.exp" check
Test case passes before the patch :- 0 for align-c++ and align-c
Test case passes after this patch:- 1134 for align-c++ and 406 for align-c.

Have a nice day ahead.

Thanks and regards,
Aditya.

[PATCH] Modify align-c/align-c++ test case for AIX

[Aditya Kamath1]

[-- Attachment #1.1: Type: text/plain, Size: 974 bytes --]

Hi all,

Please find attached the patch. {See:- 0001-Modify-align-c-align-c-test-case-for-AIX.patch}

In AIX, we have observed around 1500 failures in align-c/align-c++ testcase. The reason being this testcase shows the alignment outputs of global variables that are unused in the main (). In AIX we need to use atleast one global variable to access the same. This patch is a fix to the same. This issue is like this link pasted in the next line which we fixed before. https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60204874f5a987b164f7f194d3f96729847fe329

Kindly let me know if this is okay.

Test case command:- gmake RUNTESTFLAGS="gdb.base/align-c.exp" check
                                       gmake RUNTESTFLAGS="gdb.base/align-c++.exp" check
Test case passes before the patch :- 0 for align-c++ and align-c
Test case passes after this patch:- 1134 for align-c++ and 406 for align-c.

Have a nice day ahead.

Thanks and regards,
Aditya.

[-- Attachment #2: 0001-Modify-align-c-align-c-test-case-for-AIX.patch --]
[-- Type: application/octet-stream, Size: 1004 bytes --]

From 5daed42ecd34d0019f3a2e4828a5b014ee865452 Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
Date: Fri, 10 Mar 2023 01:22:14 -0600
Subject: [PATCH] Modify align-c/align-c++ test case for AIX

On AIX, the debugger cannot access global variables before they
they are used in any function. Hence we add this small change so
that we can test the same in AIX.
---
 gdb/testsuite/gdb.base/align.exp.tcl | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gdb/testsuite/gdb.base/align.exp.tcl b/gdb/testsuite/gdb.base/align.exp.tcl
index 6a75a14d887..f18243080ce 100644
--- a/gdb/testsuite/gdb.base/align.exp.tcl
+++ b/gdb/testsuite/gdb.base/align.exp.tcl
@@ -109,6 +109,11 @@ proc prepare_test_source_file { lang } {
     # Epilogue.
     puts $outfile {
 	int main() {
+	    #ifdef _AIX
+	    /* On AIX, the debugger cannot access global variables before they
+		they are used in the functions.  */
+		a_char_x_char++;
+	    #endif
 	    return 0;
 	}
     }
-- 
2.38.3