Re: [PATCH, libgomp] Fix chunk_size<1 for dynamic schedule

[Jakub Jelinek <jakub@redhat.com>]

On Thu, Aug 04, 2022 at 09:17:09PM +0800, Chung-Lin Tang wrote:
> On 2022/6/28 10:06 PM, Jakub Jelinek wrote:
> > On Thu, Jun 23, 2022 at 11:47:59PM +0800, Chung-Lin Tang wrote:
> > > with the way that chunk_size < 1 is handled for gomp_iter_dynamic_next:
> > > 
> > > (1) chunk_size <= -1: wraps into large unsigned value, seems to work though.
> > > (2) chunk_size == 0:  infinite loop
> > > 
> > > The (2) behavior is obviously not desired. This patch fixes this by changing
> > 
> > Why?  It is a user error, undefined behavior, we shouldn't slow down valid
> > code for users who don't bother reading the standard.
> 
> This is loop init code, not per-iteration. The overhead really isn't that much.

But still, we slow down valid code for the sake of garbage.
That is a task for sanitizers, not production libraries.
> 
> The question should be, if GCC having infinite loop behavior is reasonable,
> even if it is undefined in the spec.

Sure, it is perfectly valid implementation of the undefined behavior.
UB can leads to tons of surprising behavior, hangs etc. and this is exactly
the same category.

On Thu, Aug 04, 2022 at 01:31:50PM +0000, Koning, Paul via Gcc-patches wrote:
> I wouldn't think so.  The way I see "undefined code" is that you can't complain
> about "wrong code" produced by the compiler.  But for the compiler to malfunction
> on wrong input is an entirely differerent matter.  For one thing, it's hard to fix
> your code if the compiler fails.  How would you locate the offending source line?

The compiler isn't malfunctioning here.  It is similar to calling a library
function with bogus arguments, say memcpy with NULL source or destination or
some invalid pointer not pointing anywhere valid, etc.
The spec clearly says zero or negative chunk size is not valid, you use it,
you get what you ask for.  Furthermore, it is easy to find out when it hangs
on which construct it is and check if that construct is valid.

I'm strongly against slowing valid code for this.

If you want to implement -fsanitize=openmp and either in that case perform
checks on the generated code side or link with an instrumented version of
libgomp that explains users what errors they do, that is fine.

	Jakub