* [PATCH 0/2] Fix alignment issue flagged by Clang in nat/amd64-linux-siginfo.c @ 2021-06-05 0:14 Pedro Alves 2021-06-05 0:14 ` [PATCH 1/2] nat/amd64-linux-siginfo.c: Move align attribute from typedef to struct Pedro Alves ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Pedro Alves @ 2021-06-05 0:14 UTC (permalink / raw) To: gdb-patches Building GDB with current git Clang hits a number of problems. This fixes one of them. Pedro Alves (2): nat/amd64-linux-siginfo.c: Move align attribute from typedef to struct nat/amd64-linux-siginfo.c: Remove typedefs gdb/nat/amd64-linux-siginfo.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) base-commit: 386de171cbffa86e804057030f3d64a404279f43 -- 2.26.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] nat/amd64-linux-siginfo.c: Move align attribute from typedef to struct 2021-06-05 0:14 [PATCH 0/2] Fix alignment issue flagged by Clang in nat/amd64-linux-siginfo.c Pedro Alves @ 2021-06-05 0:14 ` Pedro Alves 2021-06-05 21:20 ` John Baldwin 2021-06-05 0:14 ` [PATCH 2/2] nat/amd64-linux-siginfo.c: Remove typedefs Pedro Alves 2021-06-06 14:41 ` [PATCH 0/2] Fix alignment issue flagged by Clang in nat/amd64-linux-siginfo.c Simon Marchi 2 siblings, 1 reply; 7+ messages in thread From: Pedro Alves @ 2021-06-05 0:14 UTC (permalink / raw) To: gdb-patches Compiling GDB with current git Clang (future 13) fails with (among other problems), this issue: $ make nat/amd64-linux-siginfo.o CXX nat/amd64-linux-siginfo.o src/gdb/nat/amd64-linux-siginfo.c:590:35: warning: passing 4-byte aligned argument to 8-byte aligned parameter 1 of 'compat_x32_siginfo_from_siginfo' may result in an unaligned pointer access [-Walign-mismatch] compat_x32_siginfo_from_siginfo ((struct compat_x32_siginfo *) inf, ^ 1 warning generated. The problem is that: - The flagged code is casting to "struct compat_x32_siginfo" pointer directly instead of to a pointer to the compat_x32_siginfo_t typedef. The called function is declared with a compat_x32_siginfo_t typedef pointer parameter. - Only the typedef has the __aligned__ attribute. Fix this by moving the attribute to the struct, so both struct and typedef have the same alignment. The next patch removes the typedefs. gdb/ChangeLog: yyyy-mm-dd Pedro Alves <pedro@palves.net> * nat/amd64-linux-siginfo.c (compat_x32_siginfo_t): Move __attribute__ __aligned__ from the typedef to the struct. --- gdb/nat/amd64-linux-siginfo.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gdb/nat/amd64-linux-siginfo.c b/gdb/nat/amd64-linux-siginfo.c index e2d2db6e112..9ff9361487a 100644 --- a/gdb/nat/amd64-linux-siginfo.c +++ b/gdb/nat/amd64-linux-siginfo.c @@ -206,7 +206,7 @@ typedef struct compat_siginfo /* For x32, clock_t in _sigchld is 64bit aligned at 4 bytes. */ typedef long __attribute__ ((__aligned__ (4))) compat_x32_clock_t; -typedef struct compat_x32_siginfo +typedef struct __attribute__ ((__aligned__ (8))) compat_x32_siginfo { int si_signo; int si_errno; @@ -263,7 +263,7 @@ typedef struct compat_x32_siginfo int _fd; } _sigpoll; } _sifields; -} compat_x32_siginfo_t __attribute__ ((__aligned__ (8))); +} compat_x32_siginfo_t; /* To simplify usage of siginfo fields. */ -- 2.26.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] nat/amd64-linux-siginfo.c: Move align attribute from typedef to struct 2021-06-05 0:14 ` [PATCH 1/2] nat/amd64-linux-siginfo.c: Move align attribute from typedef to struct Pedro Alves @ 2021-06-05 21:20 ` John Baldwin 2021-06-07 22:20 ` Pedro Alves 0 siblings, 1 reply; 7+ messages in thread From: John Baldwin @ 2021-06-05 21:20 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 6/4/21 5:14 PM, Pedro Alves wrote: > diff --git a/gdb/nat/amd64-linux-siginfo.c b/gdb/nat/amd64-linux-siginfo.c > index e2d2db6e112..9ff9361487a 100644 > --- a/gdb/nat/amd64-linux-siginfo.c > +++ b/gdb/nat/amd64-linux-siginfo.c > @@ -206,7 +206,7 @@ typedef struct compat_siginfo > /* For x32, clock_t in _sigchld is 64bit aligned at 4 bytes. */ > typedef long __attribute__ ((__aligned__ (4))) compat_x32_clock_t; > > -typedef struct compat_x32_siginfo > +typedef struct __attribute__ ((__aligned__ (8))) compat_x32_siginfo > { > int si_signo; > int si_errno; > @@ -263,7 +263,7 @@ typedef struct compat_x32_siginfo > int _fd; > } _sigpoll; > } _sifields; > -} compat_x32_siginfo_t __attribute__ ((__aligned__ (8))); > +} compat_x32_siginfo_t; > > /* To simplify usage of siginfo fields. */ Looks good to me. Note that alignas() is part of C++11, so you could perhaps spell this a bit shorter as "alignas (8)" instead. -- John Baldwin ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] nat/amd64-linux-siginfo.c: Move align attribute from typedef to struct 2021-06-05 21:20 ` John Baldwin @ 2021-06-07 22:20 ` Pedro Alves 2021-06-07 22:42 ` John Baldwin 0 siblings, 1 reply; 7+ messages in thread From: Pedro Alves @ 2021-06-07 22:20 UTC (permalink / raw) To: John Baldwin, gdb-patches Hi John, On 2021-06-05 10:20 p.m., John Baldwin wrote: > On 6/4/21 5:14 PM, Pedro Alves wrote: >> diff --git a/gdb/nat/amd64-linux-siginfo.c b/gdb/nat/amd64-linux-siginfo.c >> index e2d2db6e112..9ff9361487a 100644 >> --- a/gdb/nat/amd64-linux-siginfo.c >> +++ b/gdb/nat/amd64-linux-siginfo.c >> @@ -206,7 +206,7 @@ typedef struct compat_siginfo >> /* For x32, clock_t in _sigchld is 64bit aligned at 4 bytes. */ >> typedef long __attribute__ ((__aligned__ (4))) compat_x32_clock_t; >> -typedef struct compat_x32_siginfo >> +typedef struct __attribute__ ((__aligned__ (8))) compat_x32_siginfo >> { >> int si_signo; >> int si_errno; >> @@ -263,7 +263,7 @@ typedef struct compat_x32_siginfo >> int _fd; >> } _sigpoll; >> } _sifields; >> -} compat_x32_siginfo_t __attribute__ ((__aligned__ (8))); >> +} compat_x32_siginfo_t; >> /* To simplify usage of siginfo fields. */ > > Looks good to me. Note that alignas() is part of C++11, so you could > perhaps spell this a bit shorter as "alignas (8)" instead. > That sounded like a good idea at first, however, if we were to do that, I think we should do it to all __aligned__ attributes in the file. But note the one seen quoted above. Here again: typedef long __attribute__ ((__aligned__ (4))) compat_x32_clock_t; We can't use alignas for that, because alignas doesn't work with typedefs. GCC: gdb/nat/amd64-linux-siginfo.c:207:14: error: attribute ignored [-Werror=attributes] 207 | typedef long alignas (4) compat_x32_clock_t; | ^~~~~~~ Clang: gdb/nat/amd64-linux-siginfo.c:207:14: error: 'alignas' attribute cannot be applied to types typedef long alignas (4) compat_x32_clock_t; ^ Clang attempt 2: gdb/nat/amd64-linux-siginfo.c:207:33: error: 'alignas' attribute only applies to variables, data members and tag types typedef long compat_x32_clock_t alignas (4); ^ See answer 4 here: https://stackoverflow.com/questions/15788947/where-can-i-use-alignas-in-c11 "These are the only places where the standard says an alignment-specifier (alignas(...)) may be applied. Note that this does not include typedef declarations nor alias-declarations." So for consistency I'm leaving the struct attribute as is too. I'm going to merge the patches now. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] nat/amd64-linux-siginfo.c: Move align attribute from typedef to struct 2021-06-07 22:20 ` Pedro Alves @ 2021-06-07 22:42 ` John Baldwin 0 siblings, 0 replies; 7+ messages in thread From: John Baldwin @ 2021-06-07 22:42 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 6/7/21 3:20 PM, Pedro Alves wrote: > See answer 4 here: > > https://stackoverflow.com/questions/15788947/where-can-i-use-alignas-in-c11 > > "These are the only places where the standard says an alignment-specifier (alignas(...)) may be applied. > Note that this does not include typedef declarations nor alias-declarations." > > So for consistency I'm leaving the struct attribute as is too. > > I'm going to merge the patches now. Ah, I agree that it's good to be consistent. -- John Baldwin ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] nat/amd64-linux-siginfo.c: Remove typedefs 2021-06-05 0:14 [PATCH 0/2] Fix alignment issue flagged by Clang in nat/amd64-linux-siginfo.c Pedro Alves 2021-06-05 0:14 ` [PATCH 1/2] nat/amd64-linux-siginfo.c: Move align attribute from typedef to struct Pedro Alves @ 2021-06-05 0:14 ` Pedro Alves 2021-06-06 14:41 ` [PATCH 0/2] Fix alignment issue flagged by Clang in nat/amd64-linux-siginfo.c Simon Marchi 2 siblings, 0 replies; 7+ messages in thread From: Pedro Alves @ 2021-06-05 0:14 UTC (permalink / raw) To: gdb-patches Since GDB is written in C++ now, we don't need struct/union typedefs any more. Remove them from nat/amd64-linux-siginfo.c. gdb/ChangeLog: yyyy-mm-dd Pedro Alves <pedro@palves.net> * nat/amd64-linux-siginfo.c (union nat_sigval): Rename to ... (nat_sigval_t): ... this and remove typedef of same name. (struct nat_siginfo): Rename to ... (nat_siginfo_t): ... this and remove typedef of same name. (struct compat_sigval): Rename to ... (compat_sigval_t): ... this and remove typedef of same name. (struct compat_siginfo): Rename to ... (compat_siginfo_t): ... this and remove typedef of same name. (struct compat_x32_siginfo): Rename to ... (compat_x32_siginfo_t): ... this and remove typedef of same name. (amd64_linux_siginfo_fixup_common): Adjust. --- gdb/nat/amd64-linux-siginfo.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/gdb/nat/amd64-linux-siginfo.c b/gdb/nat/amd64-linux-siginfo.c index 9ff9361487a..342840e5085 100644 --- a/gdb/nat/amd64-linux-siginfo.c +++ b/gdb/nat/amd64-linux-siginfo.c @@ -45,13 +45,13 @@ typedef int nat_timer_t; /* For native 64-bit, clock_t in _sigchld is 64-bit. */ typedef long nat_clock_t; -typedef union nat_sigval +union nat_sigval_t { nat_int_t sival_int; nat_uptr_t sival_ptr; -} nat_sigval_t; +}; -typedef struct nat_siginfo +struct nat_siginfo_t { int si_signo; int si_errno; @@ -112,7 +112,7 @@ typedef struct nat_siginfo int _fd; } _sigpoll; } _sifields; -} nat_siginfo_t; +}; #endif /* __ILP32__ */ @@ -133,13 +133,13 @@ struct compat_timeval int tv_usec; }; -typedef union compat_sigval +union compat_sigval_t { compat_int_t sival_int; compat_uptr_t sival_ptr; -} compat_sigval_t; +}; -typedef struct compat_siginfo +struct compat_siginfo_t { int si_signo; int si_errno; @@ -201,12 +201,12 @@ typedef struct compat_siginfo int _fd; } _sigpoll; } _sifields; -} compat_siginfo_t; +}; /* For x32, clock_t in _sigchld is 64bit aligned at 4 bytes. */ typedef long __attribute__ ((__aligned__ (4))) compat_x32_clock_t; -typedef struct __attribute__ ((__aligned__ (8))) compat_x32_siginfo +struct __attribute__ ((__aligned__ (8))) compat_x32_siginfo_t { int si_signo; int si_errno; @@ -263,7 +263,7 @@ typedef struct __attribute__ ((__aligned__ (8))) compat_x32_siginfo int _fd; } _sigpoll; } _sifields; -} compat_x32_siginfo_t; +}; /* To simplify usage of siginfo fields. */ @@ -578,20 +578,20 @@ amd64_linux_siginfo_fixup_common (siginfo_t *ptrace, gdb_byte *inf, if (mode == FIXUP_32) { if (direction == 0) - compat_siginfo_from_siginfo ((struct compat_siginfo *) inf, ptrace); + compat_siginfo_from_siginfo ((compat_siginfo_t *) inf, ptrace); else - siginfo_from_compat_siginfo (ptrace, (struct compat_siginfo *) inf); + siginfo_from_compat_siginfo (ptrace, (compat_siginfo_t *) inf); return 1; } else if (mode == FIXUP_X32) { if (direction == 0) - compat_x32_siginfo_from_siginfo ((struct compat_x32_siginfo *) inf, + compat_x32_siginfo_from_siginfo ((compat_x32_siginfo_t *) inf, ptrace); else siginfo_from_compat_x32_siginfo (ptrace, - (struct compat_x32_siginfo *) inf); + (compat_x32_siginfo_t *) inf); return 1; } -- 2.26.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] Fix alignment issue flagged by Clang in nat/amd64-linux-siginfo.c 2021-06-05 0:14 [PATCH 0/2] Fix alignment issue flagged by Clang in nat/amd64-linux-siginfo.c Pedro Alves 2021-06-05 0:14 ` [PATCH 1/2] nat/amd64-linux-siginfo.c: Move align attribute from typedef to struct Pedro Alves 2021-06-05 0:14 ` [PATCH 2/2] nat/amd64-linux-siginfo.c: Remove typedefs Pedro Alves @ 2021-06-06 14:41 ` Simon Marchi 2 siblings, 0 replies; 7+ messages in thread From: Simon Marchi @ 2021-06-06 14:41 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 2021-06-04 8:14 p.m., Pedro Alves wrote: > Building GDB with current git Clang hits a number of problems. This > fixes one of them. > > Pedro Alves (2): > nat/amd64-linux-siginfo.c: Move align attribute from typedef to struct > nat/amd64-linux-siginfo.c: Remove typedefs > > gdb/nat/amd64-linux-siginfo.c | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > > base-commit: 386de171cbffa86e804057030f3d64a404279f43 > This LGTM, thanks! Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-06-07 22:43 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-05 0:14 [PATCH 0/2] Fix alignment issue flagged by Clang in nat/amd64-linux-siginfo.c Pedro Alves 2021-06-05 0:14 ` [PATCH 1/2] nat/amd64-linux-siginfo.c: Move align attribute from typedef to struct Pedro Alves 2021-06-05 21:20 ` John Baldwin 2021-06-07 22:20 ` Pedro Alves 2021-06-07 22:42 ` John Baldwin 2021-06-05 0:14 ` [PATCH 2/2] nat/amd64-linux-siginfo.c: Remove typedefs Pedro Alves 2021-06-06 14:41 ` [PATCH 0/2] Fix alignment issue flagged by Clang in nat/amd64-linux-siginfo.c Simon Marchi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).