Fix for ExecutionEngine buglet
Dale E. Martin
dale at the-martins.org
Wed Nov 13 11:37:30 PST 2013
> I assume from the description that you are using either the interpreter
> or the old JIT execution engine. If you are using the old JIT, have you
> considered migrating to MCJIT? That's not relevant to the patch, but I
> think it's a good idea. We're planning to deprecate the old JIT engine
> in a future release.
My project is currently using LLVM 3.2 but I've been reading the blog
entries about MCJIT and it's on my radar.
> As for the patch itself, it seems like it would be better to have the
> function return immediately if GA is NULL after the call to
> getMemoryForGV. We definitely don't want to increment the number of init
> bytes or the number of global variables in that case.
I can certainly update the patch to do an early return. I don't know that
in the big picture it's any different but it certainly looks cleaner.
> It would be nice if we had some way to let the caller know the method
> failed. Even better would be to figure out why the allocation is failing
> and fix it.
I do believe that the issue is that we have generated a lot of code and
data, and suddenly we're asking for a large global variable and the
currently allocated buffer is basically at the end. The memory allocator
says "we don't have space for that" and returns NULL. Since there is
current no check for the allocation failing, when it tries to initialize
this new global variable we try memcpy the initializer into address 0x0.
With this current patch, when that segv is avoided, that situation gets
recognized in the loop one level higher in the execution engine, so the
code generation fails, more memory gets allocated, and the code generation
succeeds on the next pass through.
> I'm assuming based on the fact that total program failure is
> avoidable that we aren't really out of memory.
Agreed. I'm pretty new to LLVM but my reading of the comments in
JITCodeEmiter.h led me to believe that this is the strategy - allocate a
certain amount of memory, try to generate code, if it's not enough, fail,
allocate more, and try again. I was assume that I had found a small hole
where the "did we run of out of memory?" check wasn't happening in time.
> That said, I wouldn't object to a patch that simply avoids catastrophic
> failure at this point if that's all you have time for and it is
> sufficient for your current needs.
It does seem to fix my current issue, but I'm willing to update the patch
to do a "full" early return if that's preferable.
Dale E. Martin - dale at the-martins.org
More information about the llvm-commits