[Valgrind-developers] Review: Patch 1/4 for bug 370028

[Ivo Raisr]

Please state if you have any objections to patch 1/4 attached at bug:
370028 Reduce the number of compiler warnings on MIPS platforms
https://bugs.kde.org/show_bug.cgi?id=370028

The patch itself:
https://bugsfiles.kde.org/attachment.cgi?id=104201

Aleksandar tested it extensively on mips32/64, arm32, ppc32/64, s390 and amd64.
I've tested it on amd64/Linux and sparcv9/Linux.
Works great.

I.

[Ivo Raisr]

Post by Ivo Raisr
370028 Reduce the number of compiler warnings on MIPS platforms
https://bugs.kde.org/show_bug.cgi?id=370028
https://bugsfiles.kde.org/attachment.cgi?id=104201
Aleksandar tested it extensively on mips32/64, arm32, ppc32/64, s390 and amd64.
I've tested it on amd64/Linux and sparcv9/Linux.
Works great.

I don't have any particular objection. But I don't fully understand the
issue either. How is this different from disabling -Wcast-align as we
do in configure.ac for arm? Is the intention to enable that warning on
all arches and use the new macro to silence it when we know the alignment
adds up?

Good question.
On arches which require aligned access (mips, sparc), gcc emits
dozilions of warnings with -Wcast-align.
However we don't want to disable this clo globally - that way we would
loose important compiler
diagnostic. The intention here is to use this macro only at
(preferably) few places where:
- the problem has been studied thoroughly
- and the alignment is actually ok but the compiler cannot deduce it itself

That particular place in vki-linux.h is a prime example because it
satisfies all the conditions above
and it produces 100+ warnings alone.
Other places could be possibly solved with a bit of
extending/redesigning Valgrind API.

I.

[Ivo Raisr]

Post by Ivo Raisr
On arches which require aligned access (mips, sparc), gcc emits
dozilions of warnings with -Wcast-align.
However we don't want to disable this clo globally - that way we would
loose important compiler
diagnostic. The intention here is to use this macro only at
- the problem has been studied thoroughly
- and the alignment is actually ok but the compiler cannot deduce it itself

So, is the intention on arm32 to remove this configure check eventually?
# On ARM we do not want to pass -Wcast-align as that produces loads
# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65459#c4
if test "X$VGCONF_ARCH_PRI" = "Xarm"; then
AC_SUBST([FLAG_W_CAST_ALIGN], [""])
else
AC_SUBST([FLAG_W_CAST_ALIGN], [-Wcast-align])
fi

Eventually, yes. Good point.
I.