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

[Hal Finkel via llvm-dev]

----- Original Message -----
> From: "Sanjoy Das" <sanjoy at playingwithpointers.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Chandler Carruth" <chandlerc at google.com>, "llvm-dev" <llvm-dev at lists.llvm.org>, "Philip Reames"
> <listmail at philipreames.com>, "Duncan P. N. Exon Smith" <dexonsmith at apple.com>
> Sent: Thursday, February 25, 2016 12:25:54 AM
> Subject: Re: [llvm-dev] Possible soundness issue with available_externally (split from "RFC: Add guard intrinsics")
> 
> 
> Hal Finkel wrote:
> 
>  > But it is not all optimizations that are the problem. Rather, it
>  > seems like a select few (e.g. things involving collapsing allowed
>  > non-determinism in atomics), and losing those optimizations seems
>  > better than generally losing function-attribute deduction.
> 
> If we go by the langref, then optimizations that fold undef are also
> problematic (though most C/C++ programs resulting in such IR would
> have UB in practice).

If the undef is folded to some concrete value (instead of just being propagated), then yes, I agree. We really should be propagating the undef, however, right?

> 
> I think there are non-(non-deterministic) problematic cases too.  The
> following won't happen today since `readnone` does not imply
> `safe_to_speculate`, but if we add a `safe_to_speculate` property
> some
> day:
> 
>    int foo(bool C) available_externally {
>      if (C)
>        ((int *) null)++; // UB
>      ret 42;
>    }
> 
>    void bar() {
>      if (<some cond>)
>       foo(true);
>    }
> 
> Now, normally you can just delete the `if (C)` branch in foo, and it
> would become just a `ret 42`, and would look like it is speculatable
> above the `<some cond>` check.  But if you then link with an -O0
> version, you'll have introduced UB if `<some cond>` is always false
> at
> runtime.

So this is a good point, but I'm not sure how much to generalize this example. When we add a safe_to_speculate attribute, we'll need to keep this in mind (special care must be taken in such non-definitive-definition contexts).

 -Hal

> 
> Today this won't happen since we don't speculate `readnone nounwind`
> functions, but could become a problem in the future.
> 
> -- Sanjoy
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory