New issue
Advanced search Search tips

Issue 828942 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

clang: non-conforming optimization -fmerge-all-constants is enabled by default

Project Member Reported by mka@chromium.org, Apr 4 2018

Issue description

https://bugs.llvm.org/show_bug.cgi?id=18538

The assertion in the following program should never fail because
variables with automatic storage duration in a function should be
created for each recursive invocation, and such distinct objects'
address should not compare equal.

  #include <assert.h>
  #include <stdio.h>
  int f(const int* p)
  {
    const int c[] = {12345,56789};
    assert(p != c);
    if (p == 0) { return f(c); }
    puts("Done.");
    return 0;
  }
  int main()
  {
    f(0);
  }

Clang 3.3 seems violating the rule and causes an assertion failure.
http://melpon.org/wandbox/permlink/tYmnEVjMNP33nYHr
> prog.exe: prog.c:6: int f(const int *): Assertion `p != c' failed.


I'm referring WG14 N1570.
http://www.open-std.org/jtc1/sc22/wg14/www/standards.html

From 6.2.4 p6:
> For such an object that does not have a variable length array type, its
> lifetime extends from entry into the block with which it is associated
> until execution of that block ends in any way. (Entering an enclosed
> block or calling a function suspends, but does not end, execution of the
> current block.) If the block is entered recursively, a new instance of
> the object is created each time.


6.5.9 p6
> Two pointers compare equal if and only if both are null pointers, both
> are pointers to the same object ...


If the variable is not an array, there seems no problem.
http://melpon.org/wandbox/permlink/uYeR8GbRvuQN3CMC


-fmerge-all-constants being enabled required the kernel to implement a workaround: https://lkml.org/lkml/2018/3/20/872

Not highest priority since the workaround is in place, but it should be fixed eventually, changing the default seems trivial.
 

Comment 1 by mka@chromium.org, Apr 4 2018

Components: Tools>ChromeOS-Toolchain
Owner: manojgupta@chromium.org
Status: Assigned (was: Untriaged)
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/06edb884e91542068f8f54fd4605d3e66e7cc348

commit 06edb884e91542068f8f54fd4605d3e66e7cc348
Author: Manoj Gupta <manojgupta@google.com>
Date: Mon Apr 09 22:18:59 2018

llvm-next: Cherry-pick 2 upstream commits.

Cherrypick following CLs to llvm-next:

Author:     Manoj Gupta <manojgupta@google.com>

    [Driver] Update GCC libraries detection logic for Gentoo.

    Summary:
    1. Find GCC's LDPATH from the actual GCC config file.
    2. Avoid picking libraries from a similar named tuple if the exact
       tuple is installed.

Author:     Manoj Gupta <manojgupta@google.com>

    Disable -fmerge-all-constants as default.

    Summary:
    "-fmerge-all-constants" is a non-conforming optimization and should not
    be the default. It is also causing miscompiles when building Linux
    Kernel (https://lkml.org/lkml/2018/3/20/872).

BUG= chromium:828942 
BUG= chromium:828505 

TEST=USE="llvm-next" sudo emerge llvm works.

Change-Id: I1372bc0a9a8b3475fc8bcaedee5e83bdb42f627c
Reviewed-on: https://chromium-review.googlesource.com/1003040
Commit-Ready: Manoj Gupta <manojgupta@chromium.org>
Tested-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Caroline Tice <cmtice@chromium.org>

[rename] https://crrev.com/06edb884e91542068f8f54fd4605d3e66e7cc348/sys-devel/llvm/llvm-7.0_pre326829_p20180318-r9.ebuild
[add] https://crrev.com/06edb884e91542068f8f54fd4605d3e66e7cc348/sys-devel/llvm/files/cherry/abb490ec962c6db7c2071ba9d1bd0cd60b4a1b3a.patch
[add] https://crrev.com/06edb884e91542068f8f54fd4605d3e66e7cc348/sys-devel/llvm/files/cherry/6f11d411da7302cfb1d049928a8222c649eef441.patch

Status: Verified (was: Assigned)
Marking fixed after new llvm roll.

Sign in to add a comment