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

Ranjeet Singh via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 14 12:49:46 PST 2018


Hi David,


thanks for pointing this out to me. I would like to be able to reproduce this issue, is this visible in buildbot at all ?


Thanks,

Ranjeet

________________________________
From: David Jones <dlj at google.com>
Sent: 14 December 2018 19:22:17
To: Ranjeet Singh
Cc: llvm-commits at lists.llvm.org
Subject: Re: [llvm] r348571 - [IR] Don't assume all functions are 4 byte aligned

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<mailto: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<mailto: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/0ad0ea21/attachment.html>


More information about the llvm-commits mailing list