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

[Aditya Kamath1 <Aditya.Kamath1@ibm.com>]

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