r234767 - [CodeGen] Fix crash with duplicated mangled name.

[Renato Golin]

On 16 April 2015 at 07:18, Gao, Yunzhong
<yunzhong_gao at playstation.sony.com> wrote:
> Hi Argyrios and Renato,
> Very sorry for the breakage.

No worries. I was on the EuroLLVM and should have seen it sooner.


> It looks like the LLVM test (tail-call-mem-intrinsics.ll) also appeared in previous logs:
> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/4712
> and the test was later removed.

Yes, ignore that, it was another error. That error made our bot
temporarily red, and when your patch failed, you didn't get the
warning email. That's why we must keep the bots green. :)


> The new clang test failed on ARM because a "void func(void *)" function is named like a constructor and the clang codegen
> is trying to emit a call to it. The ARMABIInfo expects the constructor to return a pointer to the class/struct, whereas the other
> targets seem to ignore the return type. I can twist the test case to make it pass on ARM, but I do not have a good idea yet
> of how to properly fix the issue.

By doing that you'll be introducing an assert that will trigger on
real code, just not on your test. I think we shouldn't do that.


> I was thinking maybe I need a way to call setInvalidDecl() when emitting the
> duplicate-mangled-name error message, so that other parts of the clang codegen can check that field. The clang::GlobalDecl
> wrapper does not allow me to modify the underlying Decl struct though.

I'm not very versed in the Clang parser, so I can't help much in that
area, sorry. What I can do, however, is help you test on ARM to make
sure your future patches don't break the behaviour on ARM.


> Renato, you mentioned that new bugs were introduced silently by this patch. Are there other bugs that I should be aware of?

No, I meant other breakages have gone in unnoticed because the bot was
already red, so the developers didn't get a warning email. Nothing to
do with your patch, don't worry.

cheers,
--renato