[PATCH] D83122: Fix crash when getVFABIMappings is called with an indirect call instruction

Francesco Petrogalli via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 13 06:41:58 PDT 2020


fpetrogalli accepted this revision.
fpetrogalli added a comment.
This revision is now accepted and ready to land.

Thank you @sanwou01 ! LGTM, with a nit: please double check those header included are needed before submitting.

Cheers,

Francesco



================
Comment at: llvm/unittests/Analysis/VectorFunctionABITest.cpp:12
 #include "llvm/IR/InstIterator.h"
+#include "llvm/IRReader/IRReader.h"
 #include "gtest/gtest.h"
----------------
sanwou01 wrote:
> fpetrogalli wrote:
> > Is this needed?
> For the definition of parseAssemblyString, yes.
Are you sure? `parseAssemblyString` is used on line 26, it works even without this include line. Can you double check this before submitting? Same for `<memory>`


================
Comment at: llvm/unittests/Analysis/VectorFunctionABITest.cpp:634
+  LLVMContext C;
+  std::unique_ptr<Module> M = parseIR(C, R"IR(
+define void @call(void () * %f) {
----------------
sanwou01 wrote:
> fpetrogalli wrote:
> > Very elegant, but is this `unique_ptr` needed? 
> "borrowed" from other unit tests, so I can only take credit for finding it. I do think the `unique_ptr` is needed: parseAssemblyString passes ownership of the Module to us, so it'd be rude to drop it on the floor.
:facepalm:, yes, I used `unique_ptr` too in the tests class `VFABIParserTest`. My bad.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83122/new/

https://reviews.llvm.org/D83122





More information about the llvm-commits mailing list