[llvm-dev] Error Calling eraseFromParent()

[mats petersson via llvm-dev]

On 8 August 2018 at 18:30, Ahmad Nouralizadeh via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> LLVM is built in Release mode. The version is 6.0.0. I think that a
> similar code worked on verison 3.9.0. It is probably a null pointer
> dereference occurring in eraseFromParent(). I checked and reconfirmed that
> the instruction had no uses. Perhaps I should rebuild LLVM. Thanks.
>
Ahmad,

I would highly recommend, at least if you are running into problems, that
debug using asserts enabled. This will (most of the time) give you a better
"explanation" as to what you've done wrong. Not every time, but for many of
the "we expect someone to get this wrong at some pont" type of problems.

Trying to figure out why something failed in a release build without
asserts is often painfully hard, because when you crash, the "reason" for
the crash is in code that has been inlined, the actual understanding of
what went wrong is several layers of calls above the crash-point.

--
Mats

>
> On Wed, Aug 8, 2018 at 9:03 PM, mayuyu.io <admin at mayuyu.io> wrote:
>
>> Hmmmm that’s strange. Do you get an assert when you build LLVM with this
>> code in Debug Mode or is it simply null pointer dereference?
>> On a side note, usually when doing transforms similar to yours I use
>> IRBuilder’s CreateXXX followed by a ReplaceInstWithInst which works
>> perfectly fine for me.
>>
>> Zhang
>>
>> 在 2018年8月9日，00:29，Ahmad Nouralizadeh <ahmadnouralizadeh at gmail.com> 写道：
>>
>> Hi. Thanks. I changed the code but the problem exists. This is my new
>> code which is again very simple:
>> ...
>> bool runOnFunction(Function &F) override {
>>
>> vector<Instruction *> dels;
>> dels.clear();
>>
>> for (inst_iterator It = inst_begin(&F), Ie = inst_end(&F); It != Ie;) {
>> Instruction *I = &*(It++);
>>
>> if (auto* op = dyn_cast<BinaryOperator>(I)) {
>> IRBuilder<NoFolder> builder(op);
>>
>> Value* lhs = op->getOperand(0);
>> Value* rhs = op->getOperand(1);
>> Value* mul = builder.CreateMul(lhs, rhs);
>>
>> for (auto& U : op->uses()) {
>>   User* user = U.getUser();
>>   user->setOperand(U.getOperandNo(), mul);
>> }
>> //I->eraseFromParent();
>> dels.push_back(I);
>> }
>> }
>>
>> for (auto &I : dels) {
>> I->eraseFromParent();
>> }
>> ...
>>
>> On Wed, Aug 8, 2018 at 6:16 PM, mayuyu.io <admin at mayuyu.io> wrote:
>>
>>> Hi:
>>> As stated in the documentation you shouldn’t modify it while iterating
>>> as this invalidates the iterator. Try save the pointers to a vector outside
>>> your loop and perform erasing in a new loop following your current one.
>>>
>>> Zhang
>>>
>>> 在 2018年8月8日，07:54，Ahmad Nouralizadeh via llvm-dev <
>>> llvm-dev at lists.llvm.org> 写道：
>>>
>>> Thanks Bjorn! But The problem is still there.
>>>
>>> On Wed, Aug 8, 2018 at 2:04 AM, Björn Pettersson A <
>>> bjorn.a.pettersson at ericsson.com> wrote:
>>>
>>>> It looks quite dangerous to erase the instruction I inside the loop
>>>> when iterating over all instructions in the function.
>>>> I guess it depends on how the range based iterator is implemented if
>>>> that works or not.
>>>>
>>>> I think that you, for example, can change the range based iteration
>>>>
>>>>    for (auto &I : instructions(F))
>>>>
>>>> into something like this
>>>>
>>>>    for (inst_iterator It = inst_begin(&F), Ie = inst_end(&F); It !=
>>>> Ie;) {
>>>>       Instruction *I = &*(It++);
>>>>       ...
>>>>     }
>>>>
>>>> That will make sure that you step the iterator before modifying the
>>>> data structure you iterate over.
>>>>
>>>> Regards,
>>>> Björn
>>>> ________________________________________
>>>> From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Ahmad
>>>> Nouralizadeh via llvm-dev <llvm-dev at lists.llvm.org>
>>>> Sent: Tuesday, August 7, 2018 22:26
>>>> To: paul.robinson at sony.com
>>>> Cc: llvm-dev at lists.llvm.org
>>>> Subject: Re: [llvm-dev] Error Calling eraseFromParent()
>>>>
>>>> The code is really simple. But I can not the reason for the
>>>> segmentation fault. I only know that the eraseFromParent() function leads
>>>> to it. The whole code is:
>>>> ...
>>>> bool runOnFunction(Function &F) override {
>>>>
>>>> for (auto &I : instructions(F)) {
>>>>
>>>> if (auto* op = dyn_cast<BinaryOperator>(&I)) {
>>>> IRBuilder<> builder(op);
>>>>
>>>> Value* lhs = op->getOperand(0);
>>>> Value* rhs = op->getOperand(1);
>>>> Value* mul = builder.CreateMul(lhs, rhs);
>>>>
>>>> for (auto& U : op->uses()) {
>>>>   User* user = U.getUser();
>>>>   user->setOperand(U.getOperandNo(), mul);
>>>> }
>>>>
>>>> I.eraseFromParent();
>>>>     }
>>>>
>>>> }
>>>> ...
>>>> And I think that the code worked well with LLVM-3.6.0 that I tested one
>>>> year ago. Now, I use LLVM-6.0.0.
>>>> Regards.
>>>>
>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>>>
>>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180808/2e2164e9/attachment.html>