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

New compiler error in beta: this constant cannot be used, attempted to read undefined bytes #54387

Closed
sdroege opened this issue Sep 20, 2018 · 11 comments
Assignees
Labels
A-const-eval Area: constant evaluation (mir interpretation) P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@sdroege
Copy link
Contributor

sdroege commented Sep 20, 2018

Since latest beta (and nightly), some of my crates have errors like the following

error: this constant cannot be used
  --> src/udpsrc.rs:47:1
   |
47 | const DEFAULT_CAPS: Option<gst::Caps> = None;
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempted to read undefined bytes

A standalone testcase replicating all the involved types can be found below (depends only on the libc crate).

I don't know if the error is valid, but in any case I don't understand what it means so at least the error message could be improved a bit :)

use std::marker;
use std::ptr;

extern crate libc;
use libc::{c_int, c_uint, c_void, size_t};

#[repr(C)]
pub struct CapsRef(ffi::GstCaps);

pub type Caps = GstRc<CapsRef>;

pub trait MiniObject {}
impl MiniObject for CapsRef {}

pub struct GstRc<T: MiniObject> {
    obj: ptr::NonNull<T>,
    borrowed: bool,
    phantom: marker::PhantomData<T>,
}

mod ffi {
    use super::*;

    #[repr(C)]
    #[derive(Copy, Clone)]
    pub struct GstCaps {
        pub mini_object: GstMiniObject,
    }

    #[repr(C)]
    #[derive(Copy, Clone)]
    pub struct GstMiniObject {
        pub type_: GType,
        pub refcount: c_int,
        pub lockstate: c_int,
        pub flags: c_uint,
        pub copy: GstMiniObjectCopyFunction,
        pub dispose: GstMiniObjectDisposeFunction,
        pub free: GstMiniObjectFreeFunction,
        pub n_qdata: c_uint,
        pub qdata: gpointer,
    }

    pub type GstMiniObjectCopyFunction =
        Option<unsafe extern "C" fn(*const GstMiniObject) -> *mut GstMiniObject>;
    pub type GstMiniObjectDisposeFunction =
        Option<unsafe extern "C" fn(*mut GstMiniObject) -> gboolean>;
    pub type GstMiniObjectFreeFunction = Option<unsafe extern "C" fn(*mut GstMiniObject)>;

    pub type gboolean = c_int;

    pub type GType = size_t;

    pub type gpointer = *mut c_void;

}

const FOO: Option<Caps> = None;

fn main() {
    let _meh = FOO;
}
@csmoe
Copy link
Member

csmoe commented Sep 20, 2018

the testcase compiles with stable-1.29.0.

@csmoe csmoe added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. regression-from-stable-to-beta Performance or correctness regression from stable to beta. A-const-eval Area: constant evaluation (mir interpretation) labels Sep 20, 2018
@pietroalbini pietroalbini removed the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Sep 20, 2018
@pietroalbini pietroalbini added this to Triaged in 1.30 regressions via automation Sep 20, 2018
@hanna-kruppe
Copy link
Contributor

Minimized:

pub struct GstRc {
    _obj: *const (),
    _borrowed: bool,
}

const FOO: Option<GstRc> = None;

fn main() {
    let _meh = FOO;
}

@pnkfelix
Copy link
Member

Visited during T-compiler meeting. Tentatively assigning to @eddyb for investigation.

@pnkfelix pnkfelix added the P-high High priority label Sep 27, 2018
@eddyb
Copy link
Member

eddyb commented Sep 27, 2018

cc @RalfJung @oli-obk I can't find anything in the validation code that's reading the wrong thing - it should be able to tell this is a None, which has 0 fields.

@AstralSorcerer
Copy link
Contributor

I've been looking into this. It seems that the issue stems from here. Specifically, because the niche-filling enum representation is used for Option<GstRc> and similar types, and the niche that gets filled is the second field (the bool), it is actually the second part of the ScalarPair that is defined, not the first.

@eddyb
Copy link
Member

eddyb commented Sep 28, 2018

Ahh, you're right! I don't know why only the second component was special-cased.

@RalfJung
Copy link
Member

Seems like @oli-obk's quest to keep Undef contained within the CTFE engine failed at last. :/

However, in this case nothing should even be trying to convert to a ConstValue. So I am not sure if that's really the problem.

@RalfJung
Copy link
Member

RalfJung commented Sep 29, 2018

Actually it looks like this is not a ScalarPair, but a Scalar being undef. At least that's what the line numbers are saying. It does not make much sense though.

EDIT: It is definitely a ScalarPair, just the line numbers seem to be wrong.

@RalfJung RalfJung assigned RalfJung and unassigned eddyb Sep 30, 2018
@RalfJung
Copy link
Member

RalfJung commented Sep 30, 2018

I am working on a fix.

EDIT: Submitted as #54693

bors added a commit that referenced this issue Oct 1, 2018
do not normalize all non-scalar constants to a ConstValue::ScalarPair

We still need `ConstValue::ScalarPair` for match handling (matching slices and strings), but that will never see anything `Undef`. For non-fat-ptr `ScalarPair`, just point to the allocation like larger data structures do.

Fixes #54387

r? @eddyb
@eddyb
Copy link
Member

eddyb commented Oct 2, 2018

Should this be closed before we get a beta backport?

@oli-obk oli-obk reopened this Oct 2, 2018
@pietroalbini pietroalbini moved this from Triaged to Backport in progress in 1.30 regressions Oct 2, 2018
@nikomatsakis nikomatsakis added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 4, 2018
@pietroalbini
Copy link
Member

Backported to beta. Closing this.

@pietroalbini pietroalbini moved this from Backport in progress to Fixed in 1.30 regressions Oct 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: constant evaluation (mir interpretation) P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
No open projects
Development

No branches or pull requests

10 participants