[llvm] r348571 - [IR] Don't assume all functions are 4 byte aligned

[David Jones via llvm-commits]

Hello,

I'm seeing test failures with this revision under MSan. The tests are
failing on use-of-uninitialized-value in ffmpeg.

You mentioned in the review that you had previously observed sanitizer
issues. Does this sound related? If it's not clear, would you be amenable
to reverting this change?

I did cleanly bisect the failure to this revision, so I'm quite confident
that something in this revision triggers the failure mode. (It may be a
pre-existing bug, though, not necessarily an issue in your changes.)

--dlj

On Fri, Dec 7, 2018 at 12:37 AM Ranjeet Singh via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: rsingh
> Date: Fri Dec  7 00:34:59 2018
> New Revision: 348571
>
> URL: http://llvm.org/viewvc/llvm-project?rev=348571&view=rev
> Log:
> [IR] Don't assume all functions are 4 byte aligned
>
> In some cases different alignments for function might be used to save
> space e.g. thumb mode with -Oz will try to use 2 byte function
> alignment. Similar patch that fixed this in other areas exists here
> https://reviews.llvm.org/D46110
>
> This was approved previously https://reviews.llvm.org/D55115 (r348215)
> but when committed it caused failures on the sanitizer buildbots when
> building llvm with clang (containing this patch). This is now fixed
> because I've added a check to see if getting the parent module returns
> null if it does then set the alignment to 0.
>
> Differential Revision: https://reviews.llvm.org/D55115
>
>
>
> Added:
>     llvm/trunk/test/Analysis/ConstantFolding/func-and-folding.ll
> Removed:
>     llvm/trunk/test/Assembler/2004-03-07-FunctionAddressAlignment.ll
> Modified:
>     llvm/trunk/lib/IR/ConstantFold.cpp
>
> Modified: llvm/trunk/lib/IR/ConstantFold.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/ConstantFold.cpp?rev=348571&r1=348570&r2=348571&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/IR/ConstantFold.cpp (original)
> +++ llvm/trunk/lib/IR/ConstantFold.cpp Fri Dec  7 00:34:59 2018
> @@ -27,6 +27,7 @@
>  #include "llvm/IR/GlobalAlias.h"
>  #include "llvm/IR/GlobalVariable.h"
>  #include "llvm/IR/Instructions.h"
> +#include "llvm/IR/Module.h"
>  #include "llvm/IR/Operator.h"
>  #include "llvm/IR/PatternMatch.h"
>  #include "llvm/Support/ErrorHandling.h"
> @@ -1077,10 +1078,10 @@ Constant *llvm::ConstantFoldBinaryInstru
>              isa<GlobalValue>(CE1->getOperand(0))) {
>            GlobalValue *GV = cast<GlobalValue>(CE1->getOperand(0));
>
> -          // Functions are at least 4-byte aligned.
> -          unsigned GVAlign = GV->getAlignment();
> -          if (isa<Function>(GV))
> -            GVAlign = std::max(GVAlign, 4U);
> +          unsigned GVAlign =
> +              GV->getParent()
> +                  ?
> GV->getPointerAlignment(GV->getParent()->getDataLayout())
> +                  : 0;
>
>            if (GVAlign > 1) {
>              unsigned DstWidth = CI2->getType()->getBitWidth();
>
> Added: llvm/trunk/test/Analysis/ConstantFolding/func-and-folding.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ConstantFolding/func-and-folding.ll?rev=348571&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/Analysis/ConstantFolding/func-and-folding.ll (added)
> +++ llvm/trunk/test/Analysis/ConstantFolding/func-and-folding.ll Fri Dec
> 7 00:34:59 2018
> @@ -0,0 +1,27 @@
> +; RUN: opt < %s -constprop -S -o - | FileCheck %s
> +
> +; Function Attrs: minsize norecurse nounwind optsize readnone
> +define dso_local void @foo1() #0 {
> +entry:
> +  ret void
> +}
> +
> +; Function Attrs: minsize norecurse nounwind optsize readnone
> +define dso_local void @foo2() align 4 {
> +entry:
> +  ret void
> +}
> +
> +; Function Attrs: minsize nounwind optsize
> +define dso_local i32 @main() local_unnamed_addr #1 {
> +entry:
> +; CHECK: ptrtoint
> +  %call = tail call i32 bitcast (i32 (...)* @process to i32 (i32)*)(i32
> and (i32 ptrtoint (void ()* @foo1 to i32), i32 2)) #3
> +; CHECK-NEXT: ptrtoint
> +  %call2 = tail call i32 bitcast (i32 (...)* @process to i32 (i32)*)(i32
> and (i32 ptrtoint (void ()* @foo2 to i32), i32 2)) #3
> +  ret i32 0
> +}
> +
> +; Function Attrs: minsize optsize
> +declare dso_local i32 @process(...) local_unnamed_addr #2
> +
>
> Removed: llvm/trunk/test/Assembler/2004-03-07-FunctionAddressAlignment.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Assembler/2004-03-07-FunctionAddressAlignment.ll?rev=348570&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/Assembler/2004-03-07-FunctionAddressAlignment.ll
> (original)
> +++ llvm/trunk/test/Assembler/2004-03-07-FunctionAddressAlignment.ll
> (removed)
> @@ -1,16 +0,0 @@
> -; RUN: llvm-as < %s | llvm-dis | not grep ptrtoint
> -; RUN: verify-uselistorder %s
> -; All of these should be eliminable
> -
> -
> -define i32 @foo() {
> -       ret i32 and (i32 ptrtoint (i32()* @foo to i32), i32 1)
> -}
> -
> -define i32 @foo2() {
> -       ret i32 and (i32 1, i32 ptrtoint (i32()* @foo2 to i32))
> -}
> -
> -define i1 @foo3() {
> -       ret i1 icmp ne (i1()* @foo3, i1()* null)
> -}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://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/20181214/12fc22e4/attachment.html>