Hi all,
I've been waiting for almost three months now to get a patch committed to LLVM
that fixes what I consider to be a fairly significant bug in the JIT
(http://llvm.org/bugs/show_bug.cgi?id=13678) but I've been having a hard time
getting traction on it. So far Duncan Sands is the only one who has given it
any attention, but as of the last time I checked it still wasn't committed and
the bug is still open.
Right now we ship our code with a patch to fix this in 3.1, I really don't
want to have to do that again for 3.2.
I appreciate the difficulties in running a project as large as LLVM, and it
looks like there are some recent structural changes that might decentralize
some of the review/commit burden, but it doesn't speak well for your project
when you have a bug with a fix and a unit test just ready to go that's not
getting into the next release.
Can anyone help me move this forward?
Hi Michael,
I'm sorry to hear that your patch went unnoticed for such a long time.
I've been through it and I know the pain, but as you said it yourself,
things often fall of the radar and with the number of emails the
commit and the dev lists have, it's not hard to lose track. Often, the
solution to this is to keep bumping and pinging your email until
someone notice it.
About the patch, I'm not sure trial and failure is the best approach
to emitting the exception table. I know GCC does some horrid things
(because of ULEBs and co. changing the final size), but I don't think
this is the problem in hand.
Anton (CCd) is the person that knows most about exception handling, he
might have a better view on what's the best approach to pre-allocate
enough memory for the table (if at all possible).
cheers,
--renato
Can anyone help me move this forward?
My diagnosis as to what stalled the review progress was that your
original review thread seems to have gotten mangled crossing over from
llvmdev to llvm-commits (little things like that can make a
difference).
I would recommend starting a fresh new thread on llvm-commits
containing a description of the patch, why it is needed, and what
kinds of testing is added (and of course, attach the patch and make
sure it applies cleanly on trunk).
The detail of the description of what the patch does and why it is
needed should be inversely proportional to the familiarity that you
expect a reviewer to have with your code (better-justified patches
require less-expert (hence more numerous & available) reviewers). Your
original mail on Aug 21 (2nd paragraph) seems to have some good
content to incorporate into the new thread.
You may also consider CC'ing any relevant developers (from git blame
or elsewhere); ideally, for a bug fix like this, you should CC the
developer that introduced the buggy code (this may require a bit of
archaeology).
-- Sean Silva
Hi Michael,
I know it can be frustrating trying to get a patch reviewed and committed. As you've seen waiting patiently doesn't generally work. Persistent pinging usually does work, but sometimes not as well as one would hope. In cases like this, you just have to be tenacious.
The specific thing I'd recommend is identifying the code owner and trying to contact him by various channels. Start by CC'ing the code owner on your pings. If that meets silence, maybe get on the LLVM IRC channel and see if you can find the code owner (or anyone else willing to review your patch) there. If that doesn't work, trying e-mailing the code owner directly until you get some kind of response.
As far as I've been able to tell, the code owner will never ignore you on purpose. Usually it's either that the code owner didn't notice your patch or the code owner did notice your patch and put it on his to-do list but is so hopelessly overwhelmed with other responsibilities that he hasn't gotten to it yet. Direct and persistent contact generally resolves these last two cases.
Including up-to-date versions of your patch whenever possible (which I see you have done) is also an excellent practice.
Your particular patch has an additional problem in that it isn't immediately obvious who the JIT code owner is. The CODE_OWNERS.TXT file might lead you to believe it's me, but it isn't. I think it might be Evan Cheng (copied). Or that may be a relatively orphaned part of the code, in which case you need to talk to Chris Lattner, who you will find is surprisingly responsive for someone with such broad responsibilities.
-Andy
Hi Michael, don't forget that Eric made some comments that you never replied to.
Since his comments seemed pretty pertinent I wasn't prepared to apply your patch
without a satisfactory response from you.
Ciao, Duncan.
Hi Duncan -
Duncan Sands wrote:
Hi Michael, don't forget that Eric made some comments that you never replied to.
Since his comments seemed pretty pertinent I wasn't prepared to apply your patch
without a satisfactory response from you.
I replied in the bug: 13678 – Exception table generation does not retry on buffer overflow
Please let me know if you need anything further.