| Message ID | 1460721604-19685-1-git-send-email-tomeu.vizoso@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 15, 2016 at 02:00:04PM +0200, Tomeu Vizoso wrote: > The variable used as loop counter in the igt_fixture macro had > unspecified value from the setjmp(3) man page quoted below. Because of > that, in certain circumstances and with -O2 and -Os, the initialization > of that variable would be eliminated and the compiler would complain of > uninitialized usage. Below can be found a snippet that reproduces the > problem with GCC 5.3.1 and 4.9.3 and the errors as printed by 5.3.1. > > "The compiler may optimize variables into registers, and longjmp() may > restore the values of other registers in addition to the stack pointer > and program counter. Consequently, the values of automatic variables > are unspecified after a call to longjmp() if they meet all the following > criteria: > > · they are local to the function that made the corresponding setjmp(3) > call; > > · their values are changed between the calls to setjmp(3) and > longjmp(); and > > · they are not declared as volatile." > > static void test(void) > { > igt_subtest_group { > igt_fixture { > } > > igt_subtest("foo") { > } > > igt_fixture { > } > } > } > > In file included from lib/intel_batchbuffer.h:8:0, > from lib/drmtest.h:39, > from lib/igt.h:27, > from tests/kms_addfb_basic.c:28: > tests/kms_addfb_basic.c: In function 'tiling_tests.isra.0': > lib/igt_core.h:110:43: warning: '__tmpint245' is used uninitialized in > this function [-Wuninitialized] > #define igt_fixture for (int igt_tokencat(__tmpint,__LINE__) = 0; \ > ^ > lib/igt_core.h:110:43: note: '__tmpint245' was declared here > #define igt_fixture for (int igt_tokencat(__tmpint,__LINE__) = 0; \ > ^ > lib/igt_core.h:148:31: note: in definition of macro '__igt_tokencat2' > #define __igt_tokencat2(x, y) x ## y > ^ > lib/igt_core.h:110:30: note: in expansion of macro 'igt_tokencat' > #define igt_fixture for (int igt_tokencat(__tmpint,__LINE__) = 0; \ > ^ > tests/kms_addfb_basic.c:245:3: note: in expansion of macro 'igt_fixture' > igt_fixture { > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> Oh nice, I never figured this out. Ther's another char tmp in the igt_subtest_f macro, maybe we should fix that up too, just for safety? Meanwhile applied your patch, thanks. -Daniel > --- > lib/igt_core.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/igt_core.h b/lib/igt_core.h > index b3fa7356e473..1b62371a7138 100644 > --- a/lib/igt_core.h > +++ b/lib/igt_core.h > @@ -107,7 +107,7 @@ void __igt_fixture_end(void) __attribute__((noreturn)); > * enumeration (e.g. when enumerating on systems without an intel gpu) such > * blocks should be annotated with igt_fixture. > */ > -#define igt_fixture for (int igt_tokencat(__tmpint,__LINE__) = 0; \ > +#define igt_fixture for (volatile int igt_tokencat(__tmpint,__LINE__) = 0; \ > igt_tokencat(__tmpint,__LINE__) < 1 && \ > __igt_fixture() && \ > (sigsetjmp(igt_subtest_jmpbuf, 1) == 0); \ > -- > 2.5.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/lib/igt_core.h b/lib/igt_core.h index b3fa7356e473..1b62371a7138 100644 --- a/lib/igt_core.h +++ b/lib/igt_core.h @@ -107,7 +107,7 @@ void __igt_fixture_end(void) __attribute__((noreturn)); * enumeration (e.g. when enumerating on systems without an intel gpu) such * blocks should be annotated with igt_fixture. */ -#define igt_fixture for (int igt_tokencat(__tmpint,__LINE__) = 0; \ +#define igt_fixture for (volatile int igt_tokencat(__tmpint,__LINE__) = 0; \ igt_tokencat(__tmpint,__LINE__) < 1 && \ __igt_fixture() && \ (sigsetjmp(igt_subtest_jmpbuf, 1) == 0); \
The variable used as loop counter in the igt_fixture macro had unspecified value from the setjmp(3) man page quoted below. Because of that, in certain circumstances and with -O2 and -Os, the initialization of that variable would be eliminated and the compiler would complain of uninitialized usage. Below can be found a snippet that reproduces the problem with GCC 5.3.1 and 4.9.3 and the errors as printed by 5.3.1. "The compiler may optimize variables into registers, and longjmp() may restore the values of other registers in addition to the stack pointer and program counter. Consequently, the values of automatic variables are unspecified after a call to longjmp() if they meet all the following criteria: · they are local to the function that made the corresponding setjmp(3) call; · their values are changed between the calls to setjmp(3) and longjmp(); and · they are not declared as volatile." static void test(void) { igt_subtest_group { igt_fixture { } igt_subtest("foo") { } igt_fixture { } } } In file included from lib/intel_batchbuffer.h:8:0, from lib/drmtest.h:39, from lib/igt.h:27, from tests/kms_addfb_basic.c:28: tests/kms_addfb_basic.c: In function 'tiling_tests.isra.0': lib/igt_core.h:110:43: warning: '__tmpint245' is used uninitialized in this function [-Wuninitialized] #define igt_fixture for (int igt_tokencat(__tmpint,__LINE__) = 0; \ ^ lib/igt_core.h:110:43: note: '__tmpint245' was declared here #define igt_fixture for (int igt_tokencat(__tmpint,__LINE__) = 0; \ ^ lib/igt_core.h:148:31: note: in definition of macro '__igt_tokencat2' #define __igt_tokencat2(x, y) x ## y ^ lib/igt_core.h:110:30: note: in expansion of macro 'igt_tokencat' #define igt_fixture for (int igt_tokencat(__tmpint,__LINE__) = 0; \ ^ tests/kms_addfb_basic.c:245:3: note: in expansion of macro 'igt_fixture' igt_fixture { Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> --- lib/igt_core.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)