[igt-dev] [PATCH 01/76] lib: Introduce typed cleanups

[Petri Latvala]

On Thu, Oct 06, 2022 at 07:28:31AM +0200, Zbigniew Kempczyński wrote:
> On Wed, Oct 05, 2022 at 10:02:57AM +0200, Mauro Carvalho Chehab wrote:
> > On Wed, 5 Oct 2022 09:45:32 +0200
> > Zbigniew Kempczyński <zbigniew.kempczynski at intel.com> wrote:
> > 
> > > > +void igt_cleanup_fd(volatile int *fd);
> > > > +#define igt_fd_t(x__) \
> > > > +	volatile int x__ cleanup_with(igt_cleanup_fd) = IGT_OUTER_SCOPE_INIT(-1)  
> > > 
> > > Why do we need volatile here? Variable after initalization (open) likely won't
> > > be touched (until exiting its scope) so what's the risk?
> > 
> > With current gcc versions, volatile is not needed, but this fails with
> > clang. The real problem is that C spec doesn't define what happens
> > with an automatic stack variable after doing a longjmp.
> 
> If I'm not wrong longjmp stays in same scope where fd is according to
> above (GOOD) examples. So I still don't feel what volatile changes in
> this case apart of directly touching the memory. I would expect that
> fd will be in same stack frame regardless using volatile keyword. 

You're assuming optimization keeps it in stack instead of sometimes
just having it in a register.

> 
> +Petri
> May you elaborate more what's wrong with clang here after removing 
> volatile?

In the 'good' example, possibly nothing. But some uses of
igt_cleanup_fd, especially being able to test it with the bad usage,
hits the issue.

There really is no way to get around the issue that assignments to
stack variables in scopes that you longjmp out of is unspecified in
the spec. Before this incorrectly becomes a story about gcc vs. clang,
the weirdness has also been seen with gcc on some particular
optimization settings.


-- 
Petri Latvala