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.

Sign up for GitHub

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

Fix native main() signature on 64bit #44906

Merged
merged 6 commits into from
Oct 1, 2017
Merged

Fix native main() signature on 64bit #44906

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6c9a4ba
rustc: Add Type::c_int()
dkl Sep 21, 2017
c2fe69b
rustc: Fix main() entry point signature on 64bit
dkl Sep 21, 2017
6c3f190
test: Check native main() signature
dkl Sep 28, 2017
a204c19
rustc: Specify c_int width for each target
dkl Sep 30, 2017
12a8bc6
rustc: Use 16bit c_int for msp430
dkl Sep 30, 2017
a4e8373
test: Update target specs test for new target-c-int-width field
dkl Sep 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions src/librustc_trans/base.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ use value::Value;
use rustc::util::nodemap::{NodeSet, FxHashMap, FxHashSet, DefIdSet};
use CrateInfo;

use libc::c_uint;
use std::any::Any;
use std::cell::RefCell;
use std::ffi::{CStr, CString};
Expand Down Expand Up @@ -692,7 +691,8 @@ fn maybe_create_entry_wrapper(ccx: &CrateContext) {
sp: Span,
rust_main: ValueRef,
use_start_lang_item: bool) {
let llfty = Type::func(&[ccx.isize_ty(), Type::i8p(ccx).ptr_to()], &ccx.isize_ty());
// Signature of native main(), corresponding to C's `int main(int, char **)`
let llfty = Type::func(&[Type::c_int(ccx), Type::i8p(ccx).ptr_to()], &Type::c_int(ccx));

if declare::get_defined_value(ccx, "main").is_some() {
// FIXME: We should be smart and show a better diagnostic here.
Expand All @@ -711,19 +711,27 @@ fn maybe_create_entry_wrapper(ccx: &CrateContext) {

debuginfo::gdb::insert_reference_to_gdb_debug_scripts_section_global(ccx, &bld);

// Params from native main() used as args for rust start function
let param_argc = get_param(llfn, 0);
let param_argv = get_param(llfn, 1);
let arg_argc = bld.intcast(param_argc, ccx.isize_ty(), true);
let arg_argv = param_argv;

let (start_fn, args) = if use_start_lang_item {
let start_def_id = ccx.tcx().require_lang_item(StartFnLangItem);
let start_instance = Instance::mono(ccx.tcx(), start_def_id);
let start_fn = callee::get_fn(ccx, start_instance);
(start_fn, vec![bld.pointercast(rust_main, Type::i8p(ccx).ptr_to()), get_param(llfn, 0),
get_param(llfn, 1)])
(start_fn, vec![bld.pointercast(rust_main, Type::i8p(ccx).ptr_to()),
arg_argc, arg_argv])
} else {
debug!("using user-defined start fn");
(rust_main, vec![get_param(llfn, 0 as c_uint), get_param(llfn, 1 as c_uint)])
(rust_main, vec![arg_argc, arg_argv])
};

let result = bld.call(start_fn, &args, None);
bld.ret(result);

// Return rust start function's result from native main()
bld.ret(bld.intcast(result, Type::c_int(ccx), true));
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/librustc_trans/type_.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ impl Type {
}
}

pub fn c_int(ccx: &CrateContext) -> Type {
Type::i32(ccx)
Copy link
Member

Choose a reason for hiding this comment

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

This doesn’t seem like a particularly good implementation of c_int. That being said, liblibc defines type c_int = i32 for everything, so it is fine, I guess?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, my thoughts exactly. It should be derived from the target context, as done for Type::isize() with target_pointer_width. But I thought that's not needed yet, because libc currently always uses i32 anyways. (does rustc support target platforms with 16bit/64bit C int?)

Copy link
Contributor

Choose a reason for hiding this comment

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

AVR uses a 16-bit int. I'll leave a comment on the tracking issue so this gets fixed if/when that gets upstreamed.

Copy link
Contributor

Choose a reason for hiding this comment

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

And MSP430 also. We currently support that target, but I don't think that needs to block this PR since this is strictly better than what we're already doing. (using i64).

Copy link
Member

@nagisa nagisa Sep 30, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

isize is 16 bit on MSP:

rust/src/librustc_back/target/msp430_none_elf.rs

Line 18 in b7041bf

target_pointer_width: "16".to_string(),

So this PR would bump the argument from i16 to i32. The registers in MSP are 16-bits, and arguments are passed in registers, so this will most likely break the calling convention.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I missed that. Going to work on a fix.

}

pub fn int_from_ty(ccx: &CrateContext, t: ast::IntTy) -> Type {
match t {
ast::IntTy::Is => ccx.isize_ty(),
Expand Down