[llvm] r348571 - [IR] Don't assume all functions are 4 byte aligned
David Jones via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 14 13:18:27 PST 2018
Hi Ranjeet --
On Fri, Dec 14, 2018 at 12:49 PM Ranjeet Singh <Ranjeet.Singh at arm.com>
wrote:
> 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 ?
>
>
>
Sorry, no... the failure I found is in an internal target, and FFmpeg isn't
tested in the sanitizer bots, AFAIK.
If it helps, this is the line that triggers the error:
https://github.com/FFmpeg/FFmpeg/blob/1edaf601f3a25b2c6d264d39078aa28ae9300615/libavcodec/utils.c#L96
And the struct it's reading:
https://github.com/FFmpeg/FFmpeg/blob/1edaf601f3a25b2c6d264d39078aa28ae9300615/libavcodec/avcodec.h#L3441
Sorry I can't be more helpful... I will keep working to reduce a test case.
--dlj
> 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> 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/c85d2173/attachment.html>
More information about the llvm-commits
mailing list