New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 676228 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 679081



Sign in to add a comment

Clang's __builtin_object_size(NULL, N) needs to be fixed

Project Member Reported by g...@chromium.org, Dec 21 2016

Issue description

See https://llvm.org/bugs/show_bug.cgi?id=23277 . Once that's fixed, we need to cherrypick it to ChromeOS and undo the workaround for https://chromium-review.googlesource.com/#/c/422748/ .
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 25 2016

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

commit e811746e2aad3a9135f800eeaaec8f8170d632d8
Author: George Burgess IV <gbiv@google.com>
Date: Wed Dec 21 20:59:59 2016

glibc: make __bos check for NULL in clang FORTIFY

Clang's implementation of __builtin_object_size may hand back 0 if
null is passed into it. This is incompatible with gcc's
__builtin_object_size implementation, which answers conservatively in
this case.

Until this is fixed in clang
(https://llvm.org/bugs/show_bug.cgi?id=23277), we should do these checks
ourselves to keep FORTIFY from being overly aggressive.

BUG= chromium:676228 
TEST=Local smoke tests pass on amd64-generic; will run cbuildbots on
oak+daisy+peppy.

Change-Id: I491c2bc09ba0b19df5c64a1ab00423e0e839972f
Reviewed-on: https://chromium-review.googlesource.com/422462
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/e811746e2aad3a9135f800eeaaec8f8170d632d8/sys-libs/glibc/files/local/glibc-2.19-clang-fortify.patch
[rename] https://crrev.com/e811746e2aad3a9135f800eeaaec8f8170d632d8/sys-libs/glibc/glibc-2.19-r14.ebuild

Blocking: 679081

Comment 3 by g...@chromium.org, Jan 10 2017

Quick update: we have an upstream code review for the fix at https://reviews.llvm.org/D28494 .
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 29 2017

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

commit e725e98a7d7a6e91c8f1e6a773d384f800432e22
Author: George Burgess IV <gbiv@google.com>
Date: Wed Mar 29 02:55:39 2017

llvm-next: cherrypick __builtin_object_size(NULL) fixes

This cherrypicks the fixes for
https://bugs.llvm.org/show_bug.cgi?id=23277.

BUG= chromium:676228 
TEST=cbuildbot falco, veyron_jaq, elm passes with `USE+=" llvm-next"`
on the top of the LLVM ebuild.

Change-Id: Ib2743b1f9bb76d0fd7c8f95b84b5018a77a5175a
Reviewed-on: https://chromium-review.googlesource.com/461295
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Luis Lozano <llozano@chromium.org>

[rename] https://crrev.com/e725e98a7d7a6e91c8f1e6a773d384f800432e22/sys-devel/llvm/llvm-4.0_pre285905-r12.ebuild
[add] https://crrev.com/e725e98a7d7a6e91c8f1e6a773d384f800432e22/sys-devel/llvm/files/cherry/0e64fc640b2c97cca42507f1039c51208808bbab.patch
[add] https://crrev.com/e725e98a7d7a6e91c8f1e6a773d384f800432e22/sys-devel/llvm/files/cherry/3479ed63a6d8ffef617a61e85f4fcf91622f9a1f.patch

Comment 5 by g...@chromium.org, Apr 27 2017

now that https://chromium-review.googlesource.com/c/488321/ is landed, the __bos(NULL) fixes exist in clang.

i'll let that change settle for a few days, then revert the workaround we made for this bug.
Project Member

Comment 6 by bugdroid1@chromium.org, May 3 2017

Comment 7 by g...@chromium.org, May 3 2017

Status: Fixed (was: Assigned)
workaround undone by https://chromium-review.googlesource.com/c/492509/ .

Comment 8 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61
Status: Verified (was: Fixed)
Closing. Please reopen it if its not fixed. Thanks!

Sign in to add a comment