Skip to content
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

Regression in From<([u8; 16], u16)> for SocketAddr #46270

Closed
dtolnay opened this issue Nov 26, 2017 · 9 comments
Closed

Regression in From<([u8; 16], u16)> for SocketAddr #46270

dtolnay opened this issue Nov 26, 2017 · 9 comments
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@dtolnay
Copy link
Member

dtolnay commented Nov 26, 2017

This used to work but the result is nondeterministic as of rustc 1.23.0-nightly (e97ba83 2017-11-25).

fn main() {
    println!("{:?}", std::net::SocketAddr::from(([1; 16], 0)));
}
  • On my computer in debug mode: V6([1400::f813:103e:ff7f:0]:0)
  • On my computer in release mode: V6([1600::1600:0:0:0]:0)
  • Expected correct answer: V6([101:101:101:101:101:101:101:101]:0)
@dtolnay
Copy link
Member Author

dtolnay commented Nov 26, 2017

It works in the previous nightly so the relevant commits are 5f44c65...e97ba83.

@kennytm kennytm added C-bug Category: This is a bug. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 26, 2017
@kennytm
Copy link
Member

kennytm commented Nov 26, 2017

The problem is in SocketAddr::new... I'm going to guess this is a miscompilation problem.

use std::net::*;
fn main() {
    let a = IpAddr::V6(Ipv6Addr::new(1,2,3,4,5,6,7,8));
    let b = SocketAddr::new(a, 9);
    println!("{:?}", a);
    println!("{:?}", b);
    assert_eq!(b.ip(), a);
}

@kennytm kennytm added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 26, 2017
@kennytm
Copy link
Member

kennytm commented Nov 26, 2017

The regression is introduced in db16292 #46008 @alexcrichton .

@kennytm kennytm added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Nov 26, 2017
@alexcrichton
Copy link
Member

@arielb1 is this related to rust-lang/llvm#96 perhaps where that LLVM patch was included in the LLVM update in db16292?

@arielb1
Copy link
Contributor

arielb1 commented Nov 26, 2017

@alexcrichton

I doubt that but I am investigating.

This looks suspicious:

; std::net::addr::SocketAddr::new
; Function Attrs: nounwind uwtable
define void @_ZN3std3net4addr10SocketAddr3new17hcf120d42abe9cbefE(%"net::addr::SocketAddr"* noalias nocapture sret dereferenceable(32), %"net::ip::IpAddr"* noalias nocapture readonly dereferenceable(20) %ip, i16 %port) unnamed_addr #2 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality !dbg !5534 {
start:
  %_10.sroa.0 = alloca [16 x i8], align 4
  %a1.sroa.0 = alloca [16 x i8], align 4
  %1 = bitcast %"net::ip::IpAddr"* %ip to i32*, !dbg !5535
  %2 = load i32, i32* %1, align 4, !dbg !5535, !range !5538
  %switch = icmp eq i32 %2, 1, !dbg !5535
  br i1 %switch, label %bb3, label %bb2, !dbg !5535

bb2:                                              ; preds = %start
  %a.sroa.0.0..sroa_idx2 = getelementptr inbounds %"net::ip::IpAddr", %"net::ip::IpAddr"* %ip, i64 0, i32 0, i64 4, !dbg !5535
  %a.sroa.0.0..sroa_cast = bitcast i8* %a.sroa.0.0..sroa_idx2 to i32*, !dbg !5535
  %a.sroa.0.0.copyload = load i32, i32* %a.sroa.0.0..sroa_cast, align 4, !dbg !5535
  %3 = tail call i16 @llvm.bswap.i16(i16 %port) #12, !dbg !5539
  %4 = bitcast %"net::addr::SocketAddr"* %0 to i32*, !dbg !5546
  store i32 0, i32* %4, align 4, !dbg !5546
  %_6.sroa.0.0..sroa_idx = getelementptr inbounds %"net::addr::SocketAddr", %"net::addr::SocketAddr"* %0, i64 0, i32 0, i64 4, !dbg !5546
  %_6.sroa.0.0..sroa_cast = bitcast i8* %_6.sroa.0.0..sroa_idx to i16*, !dbg !5546
  store i16 2, i16* %_6.sroa.0.0..sroa_cast, align 4, !dbg !5546
  %_6.sroa.4.0..sroa_idx = getelementptr inbounds %"net::addr::SocketAddr", %"net::addr::SocketAddr"* %0, i64 0, i32 0, i64 6, !dbg !5546
  %_6.sroa.4.0..sroa_cast = bitcast i8* %_6.sroa.4.0..sroa_idx to i16*, !dbg !5546
  store i16 %3, i16* %_6.sroa.4.0..sroa_cast, align 2, !dbg !5546
  %_6.sroa.5.0..sroa_idx = getelementptr inbounds %"net::addr::SocketAddr", %"net::addr::SocketAddr"* %0, i64 0, i32 0, i64 8, !dbg !5546
  %_6.sroa.5.0..sroa_cast = bitcast i8* %_6.sroa.5.0..sroa_idx to i32*, !dbg !5546
  store i32 %a.sroa.0.0.copyload, i32* %_6.sroa.5.0..sroa_cast, align 4, !dbg !5546
  %_6.sroa.6.0..sroa_idx = getelementptr inbounds %"net::addr::SocketAddr", %"net::addr::SocketAddr"* %0, i64 0, i32 0, i64 12, !dbg !5546
  %_6.sroa.6.0..sroa_cast = bitcast i8* %_6.sroa.6.0..sroa_idx to i64*, !dbg !5546
  store i64 0, i64* %_6.sroa.6.0..sroa_cast, align 4, !dbg !5546
  %.pre = getelementptr inbounds [16 x i8], [16 x i8]* %a1.sroa.0, i64 0, i64 0, !dbg !5547
  br label %bb4, !dbg !5548

bb3:                                              ; preds = %start
  %a1.sroa.0.0.sroa_idx10 = getelementptr inbounds [16 x i8], [16 x i8]* %a1.sroa.0, i64 0, i64 0, !dbg !5549
  call void @llvm.lifetime.start(i64 16, i8* nonnull %a1.sroa.0.0.sroa_idx10), !dbg !5549
  %5 = getelementptr inbounds %"net::ip::IpAddr", %"net::ip::IpAddr"* %ip, i64 0, i32 0, i64 4, !dbg !5549
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull %a1.sroa.0.0.sroa_idx10, i8* %5, i64 16, i32 4, i1 false), !dbg !5549
  %_10.sroa.0.0.sroa_idx4 = getelementptr inbounds [16 x i8], [16 x i8]* %_10.sroa.0, i64 0, i64 0, !dbg !5549
  ; @@@@ WHY ARE WE MAKING THIS CALL TO LIFETIME.START/LIFETIME.END??? @@@@
  call void @llvm.lifetime.start(i64 16, i8* nonnull %_10.sroa.0.0.sroa_idx4), !dbg !5549
  %6 = tail call i16 @llvm.bswap.i16(i16 %port) #12, !dbg !5550
  call void @llvm.lifetime.end(i64 16, i8* nonnull %_10.sroa.0.0.sroa_idx4), !dbg !5549
  %7 = bitcast %"net::addr::SocketAddr"* %0 to i32*, !dbg !5549
  store i32 1, i32* %7, align 4, !dbg !5549
  %_9.sroa.0.0..sroa_idx = getelementptr inbounds %"net::addr::SocketAddr", %"net::addr::SocketAddr"* %0, i64 0, i32 0, i64 4, !dbg !5549
  %_9.sroa.0.0..sroa_cast = bitcast i8* %_9.sroa.0.0..sroa_idx to i16*, !dbg !5549
  store i16 10, i16* %_9.sroa.0.0..sroa_cast, align 4, !dbg !5549
  %_9.sroa.4.0..sroa_idx = getelementptr inbounds %"net::addr::SocketAddr", %"net::addr::SocketAddr"* %0, i64 0, i32 0, i64 6, !dbg !5549
  %_9.sroa.4.0..sroa_cast = bitcast i8* %_9.sroa.4.0..sroa_idx to i16*, !dbg !5549
  store i16 %6, i16* %_9.sroa.4.0..sroa_cast, align 2, !dbg !5549
  %_9.sroa.5.0..sroa_idx = getelementptr inbounds %"net::addr::SocketAddr", %"net::addr::SocketAddr"* %0, i64 0, i32 0, i64 8, !dbg !5549
  %_9.sroa.5.0..sroa_cast = bitcast i8* %_9.sroa.5.0..sroa_idx to i32*, !dbg !5549
  store i32 0, i32* %_9.sroa.5.0..sroa_cast, align 4, !dbg !5549
  %_10.sroa.0.0._9.sroa.6.0..sroa_idx.sroa_idx = getelementptr inbounds %"net::addr::SocketAddr", %"net::addr::SocketAddr"* %0, i64 0, i32 0, i64 12, !dbg !5549
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %_10.sroa.0.0._9.sroa.6.0..sroa_idx.sroa_idx, i8* nonnull %_10.sroa.0.0.sroa_idx4, i64 16, i32 4, i1 false), !dbg !5549
  %_9.sroa.7.0..sroa_idx = getelementptr inbounds %"net::addr::SocketAddr", %"net::addr::SocketAddr"* %0, i64 0, i32 0, i64 28, !dbg !5549
  %_9.sroa.7.0..sroa_cast = bitcast i8* %_9.sroa.7.0..sroa_idx to i32*, !dbg !5549
  store i32 0, i32* %_9.sroa.7.0..sroa_cast, align 4, !dbg !5549
  br label %bb4, !dbg !5548

bb4:                                              ; preds = %bb3, %bb2
  %a1.sroa.0.0.sroa_idx13.pre-phi = phi i8* [ %.pre, %bb2 ], [ %a1.sroa.0.0.sroa_idx10, %bb3 ], !dbg !5547
  call void @llvm.lifetime.end(i64 16, i8* nonnull %a1.sroa.0.0.sroa_idx13.pre-phi), !dbg !5547
  ret void, !dbg !5557
}

@arielb1
Copy link
Contributor

arielb1 commented Nov 26, 2017

So LLVM does not generate the suspicious calls to llvm.lifetime.start/llvm.lifetime.end at that place with -C codegen-units=1. I'm not sure how to debug LLVM ThinLTO to find out which pass is performing this operation (-C llvm-args=-print-before-all + multiple codegen units = mixed output).

@arielb1
Copy link
Contributor

arielb1 commented Nov 26, 2017

This suspiciously looks like a duplicate of #46239 (yes I know it's caused by a separate PR, but the pervasive UB was exposed by changing LLVM's mood) - I'll check on this issue again after that PR lands.

@kennytm
Copy link
Member

kennytm commented Nov 26, 2017

The alt-build artifact from 71b21ed of #46253 can't reproduce the regression, should be fixed. Tagging E-needstest.

@kennytm kennytm added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Nov 26, 2017
@arielb1
Copy link
Contributor

arielb1 commented Nov 27, 2017

I think that PR already includes a sufficient test. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants