jrprice / Oclgrind

An OpenCL device simulator and debugger
Other
346 stars 61 forks source link

Assertion raised in clang::Sema when compiling ill-formed kernel #139

Closed ChrisCummins closed 7 years ago

ChrisCummins commented 7 years ago

Hi,

I have been batch testing OpenCL implementations, and have found that oclgrind 16.10 segfaults during compilation of some ill-formed kernels, due to an assertion in the clang::Sema destructor. See this file for a self-contained example:

http://paste.ubuntu.com/25125875/

Cheers, Chris

jrprice commented 7 years ago

I'm able to reproduce this with trunk Oclgrind built against LLVM/Clang 4.0.1, but not with LLVM/Clang trunk (now 6.0), so looks like this bug has been fixed in Clang at some point in the last few months.

Are you able to verify this?

ChrisCummins commented 7 years ago

Thanks for the quick response @jrprice! I've been running my tests on oclgrind built against LLVM 3.9. I can try and reproduce against LLVM trunk, though that may take a couple of weeks (sorry, limited access to hardware). I'll get back to you on that

ChrisCummins commented 7 years ago

It looks like I have a handful of other cases where oclgrind crashes, but the underlying cause may be LLVM/Clang. I’ll make a note of them here rather than spamming new issues, and let you know once I’ve reproduced on LLVM trunk:

jrprice commented 7 years ago

Thanks for these reports.

I believe the switch issue is actually an Oclgrind bug. It appears to be some sort of race-condition, as the assertion doesn't happen consistently, and goes away completely when running single threaded. I'll look into this.

The other two reports seem to be identical? That one is definitely a Clang bug - you can reproduce by compiling just the kernel with clang -cc1 -x cl -finclude-default-header foo.cl (which is a handy way of checking whether the issue is inside Clang or Oclgrind). I've reduced this test case further to the following reproducer:

kernel void A(global float* b)
{
  (float4)(b);
}

I'll see if there's an obvious fix in Clang that I can produce a patch for, otherwise I'll just report the bug to them.

jrprice commented 7 years ago

The switch issue should now be fixed in 23eeaa6.

I've opened a bug for the Clang/Sema assertion here: https://bugs.llvm.org/show_bug.cgi?id=33897

ChrisCummins commented 7 years ago

That's great thanks! :) I'll be sure to check clang compile before reporting any more compiler crashes. And yes sorry, copy-and-paste error, submitted the same file twice.

ChrisCummins commented 7 years ago

Sorry it took a while, but I finally got around to checking the Clang/Sema assert problem on other LLVM versions. Looks like it was fixed in 5.0.0, but can still be triggered using other code samples on trunk. I'll report to LLVM.