[Linuxptp-devel] Compiler warnings with -O2

[Miroslav Lichvar]

When compiling the linuxptp sources with -O2 (gcc 4.7.2), I get a
number of warnings. There are some "may be used uninitialized in this
function" and some "dereferencing type-punned pointer will break
strict-aliasing rules" warnings. What's the policy for warnings?
Should -O2 be used by default?

With this patch the strict-aliasing warnings are gone, but I'm not
sure if that really fixes the problem or confuses the compiler to not
detect the problem (or fixes a false positive).

--- a/phc2sys.c
+++ b/phc2sys.c
@@ -330,12 +330,14 @@ static int is_msg_mgt(struct ptp_message *msg)

static int get_mgt_id(struct ptp_message *msg)
{
- return ((struct management_tlv *) msg->management.suffix)->id;
+ struct management_tlv *mgt = (struct management_tlv *) msg->management.suffix;
+ return mgt->id;
}

static void *get_mgt_data(struct ptp_message *msg)
{
- return ((struct management_tlv *) msg->management.suffix)->data;
+ struct management_tlv *mgt = (struct management_tlv *) msg->management.suffix;
+ return mgt->data;
}


Thanks,

[Richard Cochran]

Post by Miroslav Lichvar
When compiling the linuxptp sources with -O2 (gcc 4.7.2), I get a
number of warnings. There are some "may be used uninitialized in this
function" and some "dereferencing type-punned pointer will break
strict-aliasing rules" warnings. What's the policy for warnings?
Should -O2 be used by default?

Our policy should be, no gcc warnings with -Wall. Let's get rid of the
warnings. We don't have any performance critical code or use cases,
and so I don't mind appeasing gcc, for example by adding variable
initializations, even when we know that it is safe not to.

I don't have a strong feeling about using -O2 or not. I don't really
completely trust gcc on the embedded, non-x86 ports. Maybe we should
leave this as an end user choice via EXTRA_CFLAGS.

As for the patch below, it is better style in any case. The code is
more readable if you write out pointer casting "long hand" rather than
dereferencing parenthesis casts, like ((type*)foo)->bar.

Thanks,
Richard

Post by Miroslav Lichvar
With this patch the strict-aliasing warnings are gone, but I'm not
sure if that really fixes the problem or confuses the compiler to not
detect the problem (or fixes a false positive).
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -330,12 +330,14 @@ static int is_msg_mgt(struct ptp_message *msg)
static int get_mgt_id(struct ptp_message *msg)
{
- return ((struct management_tlv *) msg->management.suffix)->id;
+ struct management_tlv *mgt = (struct management_tlv *) msg->management.suffix;
+ return mgt->id;
}
static void *get_mgt_data(struct ptp_message *msg)
{
- return ((struct management_tlv *) msg->management.suffix)->data;
+ struct management_tlv *mgt = (struct management_tlv *) msg->management.suffix;
+ return mgt->data;
}
Thanks,
--
Miroslav Lichvar
------------------------------------------------------------------------------
Own the Future-Intel(R) Level Up Game Demo Contest 2013
Rise to greatness in Intel's independent game demo contest. Compete
for recognition, cash, and the chance to get your game on Steam.
$5K grand prize plus 10 genre and skill prizes. Submit your demo
by 6/6/13. http://altfarm.mediaplex.com/ad/ck/12124-176961-30367-2
_______________________________________________
Linuxptp-devel mailing list
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

[Keller, Jacob E]

-----Original Message-----
Sent: Tuesday, April 02, 2013 10:24 AM
To: Miroslav Lichvar
Subject: Re: [Linuxptp-devel] Compiler warnings with -O2

Post by Miroslav Lichvar
When compiling the linuxptp sources with -O2 (gcc 4.7.2), I get a
number of warnings. There are some "may be used uninitialized in

this

Post by Miroslav Lichvar
function" and some "dereferencing type-punned pointer will break
strict-aliasing rules" warnings. What's the policy for warnings?
Should -O2 be used by default?

Our policy should be, no gcc warnings with -Wall. Let's get rid of the
warnings. We don't have any performance critical code or use cases,
and so I don't mind appeasing gcc, for example by adding variable
initializations, even when we know that it is safe not to.
I don't have a strong feeling about using -O2 or not. I don't really
completely trust gcc on the embedded, non-x86 ports. Maybe we
should
leave this as an end user choice via EXTRA_CFLAGS.
As for the patch below, it is better style in any case. The code is
more readable if you write out pointer casting "long hand" rather than
dereferencing parenthesis casts, like ((type*)foo)->bar.
Thanks,
Richard

Agreed to all ove the above :) I would leave -O2 off by default because we don't really have any performance critical issues, but we should appease gcc warnings for sure.

- Jake

[Miroslav Lichvar]

Post by Keller, Jacob E
Agreed to all ove the above :) I would leave -O2 off by default because we don't really have any performance critical issues, but we should appease gcc warnings for sure.

My concern is that the developers will not be using it and new
warnings will get in. The strict-aliasing warnings are in my code, I'd
try to fix them if I had seen them. Is the problem with -O2 that it
slows down the compilation too much?

-
Miroslav Lichvar

[Stephan Gatzka]

Post by Miroslav Lichvar

Agreed to all ove the above I would leave -O2 off by default because

we don't really have any performance critical issues, but we should
appease gcc warnings for sure.

I totally disagree on that. Unfortunately, gcc sometimes does not emit
warnings when compiling without optimizations. Nevertheless, the I only
find very, very seldom cases where the warnings of gcc are false
positives. So the code should compile without any warnings despite of
the optimizations (-O2 or -Os).

Post by Miroslav Lichvar
My concern is that the developers will not be using it and new
warnings will get in. The strict-aliasing warnings are in my code, I'd
try to fix them if I had seen them. Is the problem with -O2 that it
slows down the compilation too much?

And gcc is right to complain on that. Violating strict aliasing is not
allowed in C.

Regards,

Stephan

[Richard Cochran]

Post by Stephan Gatzka
I totally disagree on that. Unfortunately, gcc sometimes does not emit
warnings when compiling without optimizations. Nevertheless, the I only
find very, very seldom cases where the warnings of gcc are false
positives. So the code should compile without any warnings despite of
the optimizations (-O2 or -Os).

I would like to point out that the makefile supports external (out of
source tree) builds. So it is easy to have a build farm with all of
the various options combinations of gcc-version/arch/cflags,
especially since the code is small enough to compile in seconds.

Here is a build suite control script...

#!/bin/bash

all="x86 m68k ixp beagle"

for x in $all; do
echo $x
cd ~/build/linuxptp/$x
./build.sh
cd - > /dev/null
echo
done

and a typical build.sh...

#!/bin/sh

export ARCH=x86
export CROSS_COMPILE=
export KBUILD_OUTPUT=/home/richard/kernel/ptp_x86
export EXTRA_CFLAGS=-O2

make -f ~/git/linuxptp/makefile $1 CC=${CROSS_COMPILE}gcc DEBUG=-g

I always run the test builds before merging, and I will make sure to
add both -O2 and -Os into the mix. I invite you all to do the same.

And so I think we can leave the -Ox out of the makefile and let the
end user or distro developer decide what they would like.

Thanks,
Richard