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

Argv list not endswith NULL in stack initialization #98

Closed
mohanson opened this issue Feb 25, 2020 · 2 comments
Closed

Argv list not endswith NULL in stack initialization #98

mohanson opened this issue Feb 25, 2020 · 2 comments
Assignees

Comments

@mohanson
Copy link
Collaborator

mohanson commented Feb 25, 2020

I found that ckb-vm's argv has not endswith null, and then I did a simple test.

The C Standard 5.1.2.2.1/2 says:

argv[argc] shall be a null pointer.

so create a c program:

#include <stddef.h>

int main(int argc,  char *argv[]) {
	if (argv[argc] == NULL) {
		return 0;
	}
	return 1;
}

When built by gcc, and run on Linux, it returns "0".

When built by riscv-unknown-elf-gcc, run on ckb-vm, it returns "1".

You might add a NULL here: https://github.com/nervosnetwork/ckb-vm/blob/develop/src/machine/mod.rs#L160

@xxuejie
Copy link
Collaborator

xxuejie commented Feb 25, 2020

We could make this change but has to do it in a non-breaking way, I will give it a thought on how we can achieve this, thanks for the report

@xxuejie
Copy link
Collaborator

xxuejie commented Feb 26, 2020

Fixed in #99

@doitian doitian added this to 📬 Inbox in CKB - Issues Mar 2, 2020
@doitian doitian moved this from 📬 Inbox to 🚧 In Progress in CKB - Issues Mar 2, 2020
xxuejie added a commit to xxuejie/ckb-vm that referenced this issue Mar 6, 2020
@doitian doitian moved this from 🚧 In Progress to 🍴 Next Fork in CKB - Issues Apr 30, 2020
xxuejie added a commit to xxuejie/ckb-vm that referenced this issue Aug 19, 2020
xxuejie added a commit that referenced this issue Aug 27, 2020
* feat: Introduce VM version

Since one major use case of CKB VM is on blockchains, we cannot just
change the code in case of bugs. We have to wait for the next fork
event, only then can we upgrade the code. Hence we are introducing a
concept of versions in the VM, whenever we fix a bug that might alter
behavior, we will insert checking code to only include the bug fix in
the next VM version. This way blockchains can expect consistent VM
behavior when they lock specific VM version.

* fix: #97 #98 in version 1

* fix: #92 in version 1

* fix: Reading the last byte of memory would trigger OutOfBound error

* fix: Only compile version tests when we do have asm mode

* fix: Bench

* fix: Bug fixes for AOT mode

* fix: VM Test Suite version

* fix: Loading binary with unaligned executable page would trigger error

* fix: Replicate the same bugs in Rust interpreter as well.

* test: Add missing alignment tests

* fix: Default run method should always use the latest non-buggy code

* test: Remove unneeded tests
@xxuejie xxuejie closed this as completed Aug 27, 2020
CKB - Issues automation moved this from 🍴 Next Fork to ✅ Closed Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
CKB - Issues
  
✅ Closed
Development

No branches or pull requests

2 participants