Skip to content

[beta] Don't sanity check function pointers in vtables #53425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 26, 2018

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Aug 16, 2018

cc #53401

There's no beta nomination because the full fix (#53424) is not backportable

@rust-highfive
Copy link
Contributor

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Contributor

⚠️ Warning ⚠️

  • Pull requests are usually filed against the master branch for this repo, but this one is against beta. Please double check that you specified the right target!

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 16, 2018
@rust-highfive
Copy link
Contributor

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:05:11] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:05:11] tidy error: /checkout/src/test/ui/consts/const-eval/issue-53401.rs: missing trailing newline
[00:05:13] some tidy checks failed
[00:05:13] 
[00:05:13] 
[00:05:13] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:05:13] 
[00:05:13] 
[00:05:13] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:05:13] Build completed unsuccessfully in 0:00:53
[00:05:13] Build completed unsuccessfully in 0:00:53
[00:05:13] Makefile:79: recipe for target 'tidy' failed
[00:05:13] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:33fba5ae
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:000bcaa4:start=1534424673809213276,finish=1534424673816794007,duration=7580731
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0253b5e4
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:14299bf4
travis_time:start:14299bf4
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:08890759
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@rust-highfive
Copy link
Contributor

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[00:53:06] ....................................................................................................
[00:53:09] ..................................i.................................................................
[00:53:12] ....................................................................................................
[00:53:15] ....................................................................................................
[00:53:18] .........F..........................................................................................
[00:53:20] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:498:22
[00:53:20] failures:
[00:53:20] 
[00:53:20] ---- [compile-fail] compile-fail/union-ub-fat-ptr.rs stdout ----
[00:53:20] ---- [compile-fail] compile-fail/union-ub-fat-ptr.rs stdout ----
[00:53:20] 
[00:53:20] error: /checkout/src/test/compile-fail/union-ub-fat-ptr.rs:79: expected error not found: this constant likely exhibits undefined behavior
[00:53:20] 
[00:53:20] error: /checkout/src/test/compile-fail/union-ub-fat-ptr.rs:82: expected error not found: this constant likely exhibits undefined behavior
[00:53:20] error: 0 unexpected errors found, 2 expected errors not found
[00:53:20] status: exit code: 1
[0

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@oli-obk oli-obk added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 22, 2018
@oli-obk oli-obk changed the title Don't sanity check function pointers in vtables [beta] Don't sanity check function pointers in vtables Aug 22, 2018
tcx.mk_array(Option<fn()>),
])
*/
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, this is a bit surprising to me, but I'm gonna trust you all here =)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So...the issue is that functions with where bounds also contributed to the count, even if they were excluded because they aren't object safe and thus they shouldn't be in the vtable. the previous code was simply wrong, and fixing it fully would blow anything we could beta backport

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same patch should also land on master though as the layout is still wrong there?

@nikomatsakis
Copy link
Contributor

@bors r+ p=1

@bors
Copy link
Collaborator

bors commented Aug 24, 2018

📌 Commit bddfece has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 24, 2018
@nikomatsakis
Copy link
Contributor

Giving high priority because fixes a beta problem =)

@bors
Copy link
Collaborator

bors commented Aug 24, 2018

⌛ Testing commit bddfece with merge b9e5e6eea3f80a78338dde50596f88109f3494e7...

@bors
Copy link
Collaborator

bors commented Aug 24, 2018

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 24, 2018
@kennytm
Copy link
Member

kennytm commented Aug 25, 2018

codegen test failed on 32-bit platform.

---- [codegen] codegen\function-arguments.rs stdout ----

...

stderr:
------------------------------------------
C:\projects\rust\src/test\codegen\function-arguments.rs:123:11: error: expected string not found in input
// CHECK: @trait_borrow({}* nonnull %arg0.0, [4 x [[USIZE]]]* noalias readonly dereferenceable({{.*}}) %arg0.1)
          ^
C:\projects\rust\build\i686-pc-windows-msvc\test\codegen\function-arguments\function-arguments.ll:364:75: note: scanning from here
define void @str([0 x i8]* noalias nonnull readonly %arg0.0, i32 %arg0.1) unnamed_addr #0 {
                                                                          ^
C:\projects\rust\build\i686-pc-windows-msvc\test\codegen\function-arguments\function-arguments.ll:364:75: note: with variable "USIZE" equal to "i32"
define void @str([0 x i8]* noalias nonnull readonly %arg0.0, i32 %arg0.1) unnamed_addr #0 {
                                                                          ^
C:\projects\rust\build\i686-pc-windows-msvc\test\codegen\function-arguments\function-arguments.ll:370:2: note: possible intended match here
define void @trait_borrow({}* nonnull %arg0.0, [3 x i32]* noalias readonly dereferenceable(12) %arg0.1) unnamed_addr #0 {
 ^
------------------------------------------

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 25, 2018
@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 25, 2018
@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 26, 2018

@bors r=nikomatsakis

@bors
Copy link
Collaborator

bors commented Aug 26, 2018

📌 Commit 690c075 has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 26, 2018
@kennytm
Copy link
Member

kennytm commented Aug 26, 2018

@bors ping retry

@bors
Copy link
Collaborator

bors commented Aug 26, 2018

😪 I'm awake I'm awake

@bors
Copy link
Collaborator

bors commented Aug 26, 2018

⌛ Testing commit 690c075 with merge 49720ea...

bors added a commit that referenced this pull request Aug 26, 2018
[beta] Don't sanity check function pointers in vtables

cc #53401

There's no beta nomination because the full fix (#53424) is not backportable
@bors
Copy link
Collaborator

bors commented Aug 26, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 49720ea to beta...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants