[llvm-dev] Possible soundness issue with available_externally (split from "RFC: Add guard intrinsics")

[Sanjoy Das via llvm-dev]

On Fri, Feb 26, 2016 at 7:11 PM, Chandler Carruth <chandlerc at google.com> wrote:
> On Fri, Feb 26, 2016 at 7:08 PM Sanjoy Das <sanjoy at playingwithpointers.com>
> wrote:
>>
>> On Thu, Feb 25, 2016 at 9:59 AM, Sanjoy Das
>> <sanjoy at playingwithpointers.com> wrote:
>> > Couple of other examples:
>> >
>> >   void @foo(i32* %ptr) available_externally {
>> >     %discard = load i32, i32* %ptr
>> >   }
>> >   void bar() {
>> >     call @foo(i32* %x)
>> >   }
>> >
>> > ==>
>> >
>> >   void @foo(i32* %ptr) available_externally {
>> >   }
>> >   void bar() {
>> >     call @foo(i32* %x)
>> >   }
>> >
>> > ==>
>> >
>> >   void @foo(i32* %ptr) available_externally {
>> >   }
>> >   void bar() {
>> >     call @foo(i32* undef) ;; non optimized @foo will crash
>> >   }
>> >
>> >   ;; Similar example if @foo was dividing something by an integer
>> >   ;; argument
>> >
>> > We've actually seen the above in our VM (though back then we
>> > didn't realize that the problem was more general than the one
>> > case above).
>>
>> ^ This particular case above is already fixed in DeadArgElim:
>
>
> Fascinating. So I'm just proposing that we need to use similar logic in
> other IPO passes.

Yeah, so far I don't see any way around it.  Refinement is everywhere,
and even if we are able to audit it out *now*, keeping it out will be
a major burden.

-- Sanjoy

>
> We should probably factor it. Maybe we just need to admit mayBeOverridden in
> more cases. It's not clear that there is any meaningful delta between what
> has been discussed and just putting these cases into that set.
>
> -Chandler
>
>>
>>
>> bool DAE::RemoveDeadArgumentsFromCallers(Function &Fn)
>> {
>>   // We cannot change the arguments if this TU does not define the
>> function or
>>   // if the linker may choose a function body from another TU, even if the
>>   // nominal linkage indicates that other copies of the function have the
>> same
>>   // semantics. In the below example, the dead load from %p may not have
>> been
>>   // eliminated from the linker-chosen copy of f, so replacing %p with
>> undef
>>   // in callers may introduce undefined behavior.
>>   //
>>   // define linkonce_odr void @f(i32* %p) {
>>   //   %v = load i32 %p
>>   //   ret void
>>   // }
>>   if (!Fn.isStrongDefinitionForLinker())
>>     return false;
>>
>> -- Sanjoy



-- 
Sanjoy Das
http://playingwithpointers.com