[llvm] 608ee70 - [SCEV] Ensure SCEV does not replace aliases with their aliasees

[Philip Reames via llvm-commits]

I went and read back over the discussion in the bug, and am leaning 
towards believing this is an HWSAN bug.  If HWSAN is creating function 
with incorrect linkage, that's a problem for HWSAN, not a reason to 
break the optimizer.

Philip

On 2/24/23 07:30, Philip Reames via llvm-commits wrote:
> I'm confused by this change.  Your commit message talks about aren't 
> interposable symbols, but the code you're removing explicitly contains 
> a check for whether the symbol was interprosable, and as far as I can 
> tell, only changes behavior for symbols which *are* interprosable.
>
> Can you explain a bit further what's going on here?  I suspect you may 
> be fixing a symptom, not a root cause.
>
> Philip
>
> On 2/23/23 11:59, Leonard Chan via llvm-commits wrote:
>> Author: Leonard Chan
>> Date: 2023-02-23T19:59:25Z
>> New Revision: 608ee703e530b00130923417b6663c9f3816f889
>>
>> URL: 
>> https://github.com/llvm/llvm-project/commit/608ee703e530b00130923417b6663c9f3816f889
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/608ee703e530b00130923417b6663c9f3816f889.diff
>>
>> LOG: [SCEV] Ensure SCEV does not replace aliases with their aliasees
>>
>> Passes in general shouldn't replace an alias with the aliasee (see
>> https://reviews.llvm.org/D66606). This can lead to situations where a
>> linkonce_odr symbol (which could be interposable if lowered to weak
>> linkage) can be replaced with a local aliasee which won't be
>> interposable. SVEC does this when the function is invoked by
>> FunctionPass Manager -> Loop Pass Manager -> Induction Variable Users in
>> the codegen pipeline. This was found in hwasan instrumented code where a
>> linonce_odr alias was replaced with its private aliasee.
>>
>> This fixes the bug descriped at
>> https://github.com/llvm/llvm-project/issues/60668.
>>
>> Differential Revision: https://reviews.llvm.org/D144035
>>
>> Added:
>>      llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
>>
>> Modified:
>>      llvm/lib/Analysis/ScalarEvolution.cpp
>>
>> Removed:
>>
>>
>> ################################################################################ 
>>
>> diff  --git a/llvm/lib/Analysis/ScalarEvolution.cpp 
>> b/llvm/lib/Analysis/ScalarEvolution.cpp
>> index 5c2c12c7d0591..93c9349b06b82 100644
>> --- a/llvm/lib/Analysis/ScalarEvolution.cpp
>> +++ b/llvm/lib/Analysis/ScalarEvolution.cpp
>> @@ -7429,10 +7429,6 @@ ScalarEvolution::getOperandsToCreate(Value *V, 
>> SmallVectorImpl<Value *> &Ops) {
>>     } else if (ConstantInt *CI = dyn_cast<ConstantInt>(V))
>>       return getConstant(CI);
>>     else if (GlobalAlias *GA = dyn_cast<GlobalAlias>(V)) {
>> -    if (!GA->isInterposable()) {
>> -      Ops.push_back(GA->getAliasee());
>> -      return nullptr;
>> -    }
>>       return getUnknown(V);
>>     } else if (!isa<ConstantExpr>(V))
>>       return getUnknown(V);
>> @@ -7619,7 +7615,7 @@ const SCEV *ScalarEvolution::createSCEV(Value 
>> *V) {
>>     } else if (ConstantInt *CI = dyn_cast<ConstantInt>(V))
>>       return getConstant(CI);
>>     else if (GlobalAlias *GA = dyn_cast<GlobalAlias>(V))
>> -    return GA->isInterposable() ? getUnknown(V) : 
>> getSCEV(GA->getAliasee());
>> +    return getUnknown(V);
>>     else if (!isa<ConstantExpr>(V))
>>       return getUnknown(V);
>>
>> diff  --git a/llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll 
>> b/llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
>> new file mode 100644
>> index 0000000000000..d4d0b7afbdbc8
>> --- /dev/null
>> +++ b/llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
>> @@ -0,0 +1,19 @@
>> +; NOTE: Assertions have been autogenerated by 
>> utils/update_analyze_test_checks.py
>> +; RUN: opt -passes='print<scalar-evolution>' -disable-output %s 2>&1 
>> | FileCheck %s
>> +
>> +target datalayout = 
>> "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
>> +target triple = "aarch64-unknown-linux-gnu"
>> +
>> + at glob.private = private constant [32 x i32] zeroinitializer
>> + at glob = linkonce_odr hidden alias [32 x i32], inttoptr (i64 add (i64 
>> ptrtoint (ptr @glob.private to i64), i64 1234) to ptr)
>> +
>> +define hidden void @func(i64 %idx) local_unnamed_addr {
>> +; CHECK-LABEL: 'func'
>> +; CHECK-NEXT:  Classifying expressions for: @func
>> +; CHECK-NEXT:    %arrayidx = getelementptr inbounds [32 x i32], ptr 
>> @glob, i64 0, i64 %idx
>> +; CHECK-NEXT:    --> ((4 * %idx) + @glob) U: [0,-1) S: 
>> [-9223372036854775808,9223372036854775807)
>> +; CHECK-NEXT:  Determining loop execution counts for: @func
>> +;
>> +  %arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 0, 
>> i64 %idx
>> +  ret void
>> +}
>>
>>
>>          _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits