| Message ID | 20230808060815.9001-3-kariem.taha2.7@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Implement the stat system calls for FreeBSD. | expand |
On 8/7/23 23:07, Karim Taha wrote: > +/* > + * Tricky points: > + * - Use __builtin_choose_expr to avoid type promotion from ?:, > + * - Invalid sizes result in a compile time error stemming from > + * the fact that abort has no parameters. > + * - It's easier to use the endian-specific unaligned load/store > + * functions than host-endian unaligned load/store plus tswapN. > + * - The pragmas are necessary only to silence a clang false-positive > + * warning: see https://bugs.llvm.org/show_bug.cgi?id=39113 . > + * - We have to disable -Wpragmas warnings to avoid a complaint about > + * an unknown warning type from older compilers that don't know about > + * -Waddress-of-packed-member. > + * - gcc has bugs in its _Pragma() support in some versions, eg > + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83256 -- so we only > + * include the warning-suppression pragmas for clang Perhaps s/in some versions/prior to gcc-13/ ? At least that's what the bugzilla says, and it will help when auditing for compiler versions in a few years when gcc-12 is EOL. Either way, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On Tue, Aug 8, 2023 at 2:50 PM Richard Henderson < richard.henderson@linaro.org> wrote: > On 8/7/23 23:07, Karim Taha wrote: > > +/* > > + * Tricky points: > > + * - Use __builtin_choose_expr to avoid type promotion from ?:, > > + * - Invalid sizes result in a compile time error stemming from > > + * the fact that abort has no parameters. > > + * - It's easier to use the endian-specific unaligned load/store > > + * functions than host-endian unaligned load/store plus tswapN. > > + * - The pragmas are necessary only to silence a clang false-positive > > + * warning: see https://bugs.llvm.org/show_bug.cgi?id=39113 . > > + * - We have to disable -Wpragmas warnings to avoid a complaint about > > + * an unknown warning type from older compilers that don't know about > > + * -Waddress-of-packed-member. > > + * - gcc has bugs in its _Pragma() support in some versions, eg > > + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83256 -- so we only > > + * include the warning-suppression pragmas for clang > > Perhaps s/in some versions/prior to gcc-13/ ? > At least that's what the bugzilla says, and it > will help when auditing for compiler versions > in a few years when gcc-12 is EOL. > Looking at this now, This bug is in clang 7, from 2018. For FreeBSD, we don't support anything older than clang 12 or 13 However, the bug still exists in clang 16, the latest. I believe this was also copied verbatim from linux-user, so let's leave it and then make this one of the common things as a followup... ok? Warner Either way, > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > r~ >
On 8/8/23 19:56, Warner Losh wrote: > Looking at this now, This bug is in clang 7, from 2018. For FreeBSD, we don't support > anything older than clang 12 or 13 However, the bug still exists in clang 16, the latest. > I believe this was also copied verbatim from linux-user, so let's leave it and then make > this one of the common things as a followup... ok? Ah. In which case this should simply be moved to qemu/compiler.h. r~
On Tue, Aug 8, 2023 at 9:05 PM Richard Henderson < richard.henderson@linaro.org> wrote: > On 8/8/23 19:56, Warner Losh wrote: > > Looking at this now, This bug is in clang 7, from 2018. For FreeBSD, we > don't support > > anything older than clang 12 or 13 However, the bug still exists in > clang 16, the latest. > > I believe this was also copied verbatim from linux-user, so let's leave > it and then make > > this one of the common things as a followup... ok? > > Ah. In which case this should simply be moved to qemu/compiler.h. > OK. That we can do. I'll look into it. Warner > > r~ >
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h index edf9602f9b..dfdfa8dd67 100644 --- a/bsd-user/qemu.h +++ b/bsd-user/qemu.h @@ -276,6 +276,37 @@ static inline bool access_ok(int type, abi_ulong addr, abi_ulong size) * These are usually used to access struct data members once the struct has been * locked - usually with lock_user_struct(). */ + +/* + * Tricky points: + * - Use __builtin_choose_expr to avoid type promotion from ?:, + * - Invalid sizes result in a compile time error stemming from + * the fact that abort has no parameters. + * - It's easier to use the endian-specific unaligned load/store + * functions than host-endian unaligned load/store plus tswapN. + * - The pragmas are necessary only to silence a clang false-positive + * warning: see https://bugs.llvm.org/show_bug.cgi?id=39113 . + * - We have to disable -Wpragmas warnings to avoid a complaint about + * an unknown warning type from older compilers that don't know about + * -Waddress-of-packed-member. + * - gcc has bugs in its _Pragma() support in some versions, eg + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83256 -- so we only + * include the warning-suppression pragmas for clang + */ +#ifdef __clang__ +#define PRAGMA_DISABLE_PACKED_WARNING \ + _Pragma("GCC diagnostic push"); \ + _Pragma("GCC diagnostic ignored \"-Wpragmas\""); \ + _Pragma("GCC diagnostic ignored \"-Waddress-of-packed-member\"") + +#define PRAGMA_REENABLE_PACKED_WARNING \ + _Pragma("GCC diagnostic pop") + +#else +#define PRAGMA_DISABLE_PACKED_WARNING +#define PRAGMA_REENABLE_PACKED_WARNING +#endif + #define __put_user(x, hptr)\ ({\ int size = sizeof(*hptr);\