[llvm] r369160 - [Attributor] Fix: Do not partially resolve returned calls.

David Jones via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 19 00:15:27 PDT 2019


FYI: reverted as r369236.

On Sun, Aug 18, 2019 at 11:44 PM David Jones <dlj at google.com> wrote:

> FYI, this revision appears to cause some tests to fail under UBSAN.
>
> Example test output:
> llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll
>
> [...]/llvm/lib/Transforms/IPO/Attributor.cpp:891:17: runtime error: load
> of value 168, which is not a valid value for type 'bool'
>     #0 [...] in
> AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::$_9::operator()(llvm::Value&,
> AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&, bool) const
> [...]/llvm/lib/Transforms/IPO/Attributor.cpp:891:17
>     #1 [...] in void llvm::function_ref<void (llvm::Value&,
> AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&,
> bool)>::callback_fn<AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::$_9>(long,
> llvm::Value&,
> AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&, bool)
> [...]/llvm/include/llvm/ADT/STLExtras.h:125:12
>     #2 [...] in llvm::function_ref<void (llvm::Value&,
> AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&,
> bool)>::operator()(llvm::Value&,
> AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&, bool) const
> [...]/llvm/include/llvm/ADT/STLExtras.h:142:12
>     #3 [...] in bool genericValueTraversal<llvm::AAReturnedValues,
> AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState>(llvm::Attributor&,
> llvm::IRPosition, llvm::AAReturnedValues const&,
> AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&,
> llvm::function_ref<void (llvm::Value&,
> AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&, bool)>
> const&, int) [...]/llvm/lib/Transforms/IPO/Attributor.cpp:207:5
>     #4 [...] in
> AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::$_2::operator()(llvm::Value&,
> AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&) const
> [...]/llvm/lib/Transforms/IPO/Attributor.cpp:902:12
>     #5 [...] in AAReturnedValuesImpl::updateImpl(llvm::Attributor&)
> [...]/llvm/lib/Transforms/IPO/Attributor.cpp:982:9
>     #6 [...] in llvm::AbstractAttribute::update(llvm::Attributor&)
> [...]/llvm/lib/Transforms/IPO/Attributor.cpp:264:16
>     #7 [...] in llvm::Attributor::run()
> [...]/llvm/lib/Transforms/IPO/Attributor.cpp:2543:17
>     #8 [...] in runAttributorOnModule(llvm::Module&)
> [...]/llvm/lib/Transforms/IPO/Attributor.cpp:2881:12
>     #9 [...] in (anonymous
> namespace)::MPPassManager::runOnModule(llvm::Module&)
> [...]/llvm/lib/IR/LegacyPassManager.cpp:1750:27
>     #10 [...] in llvm::legacy::PassManagerImpl::run(llvm::Module&)
> [...]/llvm/lib/IR/LegacyPassManager.cpp:1863:44
>     #11 [...] in main [...]/llvm/tools/opt/opt.cpp:892:12
>     #12 [...] in __libc_start_main (/usr/grte/v4/lib64/libc.so.6+[...])
>     #13 [...] in _start
> /usr/grte/v4/debug-src/src/csu/../sysdeps/x86_64/start.S:108
>
> SUMMARY: UndefinedBehaviorSanitizer: invalid-bool-load
> [...]/llvm/lib/Transforms/IPO/Attributor.cpp:891:17 in
> FileCheck error: '-' is empty.
>
>
>
> (The test llvm/test/Transforms/FunctionAttrs/arg_returned.ll fails in a
> similar way)
>
>
> On Fri, Aug 16, 2019 at 2:58 PM Johannes Doerfert via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: jdoerfert
>> Date: Fri Aug 16 14:59:52 2019
>> New Revision: 369160
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=369160&view=rev
>> Log:
>> [Attributor] Fix: Do not partially resolve returned calls.
>>
>> By partially resolving returned calls we did not record that they were
>> not fully resolved which caused odd behavior down the line. We could
>> also end up with some, but not all, returned values of the callee in the
>> returned values map of the caller, another odd behavior we want to
>> avoid.
>>
>> Modified:
>>     llvm/trunk/lib/Transforms/IPO/Attributor.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/IPO/Attributor.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/Attributor.cpp?rev=369160&r1=369159&r2=369160&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/IPO/Attributor.cpp (original)
>> +++ llvm/trunk/lib/Transforms/IPO/Attributor.cpp Fri Aug 16 14:59:52 2019
>> @@ -943,12 +943,35 @@ ChangeStatus AAReturnedValuesImpl::updat
>>                        << static_cast<const AbstractAttribute &>(RetValAA)
>>                        << "\n");
>>
>> -    // If we know something but not everyting about the returned values,
>> keep
>> -    // track of that too. Hence, remember transitively unresolved calls.
>> -    UnresolvedCalls.insert(RetValAA.getUnresolvedCalls().begin(),
>> -                           RetValAA.getUnresolvedCalls().end());
>> +    // Do not try to learn partial information. If the callee has
>> unresolved
>> +    // return values we will treat the call as unresolved/opaque.
>> +    auto &RetValAAUnresolvedCalls = RetValAA.getUnresolvedCalls();
>> +    if (!RetValAAUnresolvedCalls.empty()) {
>> +      UnresolvedCalls.insert(CB);
>> +      continue;
>> +    }
>> +
>> +    // Now check if we can track transitively returned values. If
>> possible, thus
>> +    // if all return value can be represented in the current scope, do
>> so.
>> +    bool Unresolved = false;
>> +    for (auto &RetValAAIt : RetValAA.returned_values()) {
>> +      Value *RetVal = RetValAAIt.first;
>> +      if (isa<Argument>(RetVal) || isa<CallBase>(RetVal) ||
>> +          isa<Constant>(RetVal))
>> +        continue;
>> +      // Anything that did not fit in the above categories cannot be
>> resolved,
>> +      // mark the call as unresolved.
>> +      LLVM_DEBUG(dbgs() << "[AAReturnedValues] transitively returned
>> value "
>> +                           "cannot be translated: "
>> +                        << *RetVal << "\n");
>> +      UnresolvedCalls.insert(CB);
>> +      Unresolved = true;
>> +      break;
>> +    }
>> +
>> +    if (Unresolved)
>> +      continue;
>>
>> -    // Now track transitively returned values.
>>      for (auto &RetValAAIt : RetValAA.returned_values()) {
>>        Value *RetVal = RetValAAIt.first;
>>        if (Argument *Arg = dyn_cast<Argument>(RetVal)) {
>> @@ -967,12 +990,6 @@ ChangeStatus AAReturnedValuesImpl::updat
>>          NewRVsMap[RetVal].insert(It.second.begin(), It.second.end());
>>          continue;
>>        }
>> -      // Anything that did not fit in the above categories cannot be
>> resolved,
>> -      // mark the call as unresolved.
>> -      LLVM_DEBUG(dbgs() << "[AAReturnedValues] transitively returned
>> value "
>> -                           "cannot be translated: "
>> -                        << *RetVal << "\n");
>> -      UnresolvedCalls.insert(CB);
>>      }
>>    }
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190819/510ffa1a/attachment.html>


More information about the llvm-commits mailing list