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

Issue 718867 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Mac
Pri: 1
Type: Bug

Blocked on:
issue 714769



Sign in to add a comment

Issues with CFI builds

Project Member Reported by aarya@google.com, May 5 2017

Issue description

Issues:

1) Something regressed in CFI, causing it to throw bad stacks. E.g. https://clusterfuzz.com/v2/testcase-detail/6515467650072576 (currently open_
others - https://bugs.chromium.org/p/chromium/issues/list?can=1&q=sandbox%3A%3Abpf_dsl&colspec=ID+Pri+M+Stars+ReleaseBlock+Component+Status+Owner+Summary+OS+Modified&x=m&y=releaseblock&cells=ids
	
../../build/linux/debian_jessie_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/shared_ptr_base.h:396:21: runtime error: control flow integrity check for type sandbox::bpf_dsl::(anonymous namespace)::ReturnResultExprImpl failed during cast to unrelated type (vtable address 0x7f8ee6a5c990)

All of them have "Bad-cast to sandbox::bpf_dsl" in common. The remaining bootom stack frames look legit and fine, but this main one which is used for testcase deduplication is bad. Any idea what is causing this.

2) Builds take too long to link. Overall time was > 1 hr locally. Are there any ways to optimize this. We want developer to reproduce clusterfuzz testcase smoothly using our new https://github.com/google/clusterfuzz-tools reproduce tool.
 

Comment 1 by aarya@google.com, May 5 2017

Cc: thakis@chromium.org
Couldn't (1) be due to a source change? Why do you think CFI itself broke?

(2) is addressed by thinlto, tracked in  issue 660216 .
Regarding (1), We didn't see sandbox::bpf_dsl across any other sanitizer builds stacktraces, only happening in CFI builds. So, suspecting something is wrong there.

Thanks for link to (2)

Comment 4 by p...@chromium.org, May 5 2017

I think this is a bad cast in libstdc++. Here is the relevant code in shared_ptr_base.h:

      template<typename... _Args>
        _Sp_counted_ptr_inplace(_Alloc __a, _Args&&... __args)
        : _M_impl(__a)
        {
          _M_impl._M_ptr = static_cast<_Tp*>(static_cast<void*>(&_M_storage)); // <<< line 396 here
          // _GLIBCXX_RESOLVE_LIB_DEFECTS
          // 2070.  allocate_shared should use allocator_traits<A>::construct
          allocator_traits<_Alloc>::construct(__a, _M_impl._M_ptr,
              std::forward<_Args>(__args)...); // might throw
        }

This code appears to be casting an uninitialized storage pointer and then constructing an object over it (see http://cplusplus.github.io/LWG/lwg-active.html#2070 if you are curious why). I guess that bpf_dsl is the only user of shared_ptr that happens to call this ctor.

I will add a blacklist entry for the ctor.
Cc: mdempsky@chromium.org
http://chromium-cpp.appspot.com/ bans shared_ptr, why does that code use it :-/

Comment 6 by p...@chromium.org, May 5 2017

Blockedon: 714769
Because this is an issue with the standard library I have added the blacklist entry to LLVM's CFI blacklist in r302272. So this is now blocked on the next clang roll.
> why does that code use it

So sandbox/linux could be reused more easily outside of Chromium.

Comment 8 by aarya@google.com, May 5 2017

 Issue 716983  has been merged into this issue.
Project Member

Comment 9 by ClusterFuzz, May 5 2017

Labels: OS-Linux
Project Member

Comment 10 by ClusterFuzz, May 5 2017

Labels: OS-Mac
 Issue 722031  has been merged into this issue.

Comment 12 by aarya@google.com, May 17 2017

 Issue 723209  has been merged into this issue.
 Issue 723212  has been merged into this issue.
 Issue 723215  has been merged into this issue.

Comment 13 by aarya@google.com, May 17 2017

 Issue 723269  has been merged into this issue.
 Issue 723270  has been merged into this issue.
 Issue 723271  has been merged into this issue.
 Issue 723273  has been merged into this issue.
 Issue 723275  has been merged into this issue.
 Issue 723277  has been merged into this issue.
 Issue 723281  has been merged into this issue.
 Issue 723282  has been merged into this issue.

Comment 14 by aarya@google.com, May 17 2017

Dont understand how this firehose lighted up today, too many such reports, disabling the entire signature on ClusterFuzz.
Project Member

Comment 15 by ClusterFuzz, May 17 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 4856354354495488 is verified as fixed, so closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Comment 16 by aarya@google.com, May 17 2017

Labels: -ClusterFuzz-Verified ClusterFuzz-Wrong
Status: Assigned (was: Verified)

Comment 17 by aarya@google.com, May 17 2017

 Issue 723304  has been merged into this issue.
 Issue 723329  has been merged into this issue.
 Issue 723333  has been merged into this issue.
 Issue 723341  has been merged into this issue.
 Issue 723343  has been merged into this issue.
 Issue 723349  has been merged into this issue.
 Issue 723403  has been merged into this issue.
 Issue 723425  has been merged into this issue.
 Issue 723441  has been merged into this issue.
 Issue 723443  has been merged into this issue.
 Issue 723457  has been merged into this issue.
 Issue 723471  has been merged into this issue.
 Issue 723476  has been merged into this issue.
 Issue 723486  has been merged into this issue.
 Issue 723512  has been merged into this issue.
 Issue 723521  has been merged into this issue.
 Issue 723527  has been merged into this issue.
 Issue 723571  has been merged into this issue.
 Issue 723596  has been merged into this issue.
 Issue 723666  has been merged into this issue.

Comment 18 by aarya@google.com, May 17 2017

 Issue 723675  has been merged into this issue.

Comment 19 by aarya@google.com, May 17 2017

 Issue 723715  has been merged into this issue.
 Issue 723716  has been merged into this issue.
 Issue 723717  has been merged into this issue.
 Issue 723718  has been merged into this issue.
 Issue 723722  has been merged into this issue.

Comment 20 by aarya@google.com, May 17 2017

 Issue 723726  has been merged into this issue.
Project Member

Comment 21 by ClusterFuzz, May 17 2017

Labels: OS-Android
c#16, c#16, c#19, were all due to bad dcheck enabling everywhere.

All other sanitizer builders have caught up with the revert of dchecks

Peter, please fix CFI builder soon so that revert cl is uptaken.
https://chromium.googlesource.com/chromium/src/+/8cc767cb0d1c65af6d5a36858c9633c929b7e4cd
Cc: gab@chromium.org
gab for comment 22

Comment 24 by p...@chromium.org, May 17 2017

https://codereview.chromium.org/2892643002/ should fix the CFI bots.

Comment 25 by aarya@google.com, May 17 2017

 Issue 723771  has been merged into this issue.
 Issue 723773  has been merged into this issue.
 Issue 723774  has been merged into this issue.
 Issue 723775  has been merged into this issue.
 Issue 723776  has been merged into this issue.
 Issue 723777  has been merged into this issue.
 Issue 723778  has been merged into this issue.
 Issue 723780  has been merged into this issue.

Comment 26 by gab@chromium.org, May 17 2017

Is this the same as  issue 723758 ?
No, this is a separate issue that was made more visible by the enabling of DCHECKs.

Comment 28 by gab@chromium.org, May 17 2017

I see, so enabling DCHECKs helped find a real issue in this case?
We already knew about this issue. Just that crashing in DCHECKS in more places caused this to happen a lot more often.

Comment 30 by aarya@google.com, May 17 2017

 Issue 723884  has been merged into this issue.
 Issue 723911  has been merged into this issue.
 Issue 723914  has been merged into this issue.
FYI, r303273 has been rolled as per https://codereview.chromium.org/2884383004
Status: Fixed (was: Assigned)
Roll worked.
Status: Assigned (was: Fixed)
Roll is reverted, reopening till it relands.

Comment 34 by aarya@google.com, May 22 2017

Status: Fixed (was: Assigned)
Looks like roll worked this time.
Project Member

Comment 35 by ClusterFuzz, Jul 14 2017

Labels: Needs-Feedback
ClusterFuzz testcase 6719543983734784 is still reproducing on tip-of-tree build (trunk).

Please re-test your fix against this testcase and if the fix was incorrect or incomplete, please re-open the bug. Otherwise, ignore this notification and add ClusterFuzz-Wrong label.
Labels: -ClusterFuzz-Wrong
We have made a bunch of changes on ClusterFuzz side, so resetting ClusterFuzz-Wrong label.

Sign in to add a comment