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

cargo test segfaulting on i386 #155

Closed
plugwash opened this issue Oct 31, 2020 · 9 comments · Fixed by #156
Closed

cargo test segfaulting on i386 #155

plugwash opened this issue Oct 31, 2020 · 9 comments · Fixed by #156

Comments

@plugwash
Copy link

Debianhas recently upgraded rust-bindgen to 0.55.1 in unstable. To keep dependencies satisfiable the update of bindgen required updating rust-onig-sys as well and it was upgraded to 69.5.1.

However, after the update of bindgen onig-sys it was discovered that the tests for onig started failing on i386 and armhf and this is currently blocking the testing migration of the new rust-bindgen package. On i386 the tests fail with a segfault and on armhf they faile with a bus error. I performed further testing on i386.

First I tried updating to the latest version of the onig crate and the error persisted.

I then tried doing a test outside of the "debcargo" environment, I first tried using
an upstream git checkout but that failed to find it's embedded copy of libonig
so I decided to use cargo clone instead.

cargo install cargo clone
cargo clone onig
cd onig
cargo test

And again that failed in the same way

running 42 tests
test buffers::tests::byte_buffer_create ... ok
test buffers::tests::rust_bytes_encoding_is_ascii ... ok
test buffers::tests::rust_string_ptr_offsets_are_valid ... ok
test buffers::tests::rust_bytes_ptr_offsets_are_valid ... ok
test buffers::tests::rust_string_encoding_is_utf8 ... ok
test find::tests::test_captures_iter ... ok
test find::tests::test_find_iter ... ok
test find::tests::test_find_iter_empty_after_match ... ok
test find::tests::test_find_iter_one_zero_length ... ok
test find::tests::test_regex_captures ... ok
test find::tests::test_regex_subcaptures ... ok
test find::tests::test_regex_subcapturespos ... ok
test match_param::test::create_default_match_param ... ok
test find::tests::test_zero_length_matches_jumps_past_match_location ... ok
test match_param::test::set_max_stack_size_limit ... ok
test match_param::test::set_retry_limit_in_match ... ok
test find::tests::test_find_iter_many_zero_length ... ok
error: test failed, to rerun pass '--lib'

Caused by:
  process didn't exit successfully: `/onig/target/debug/deps/onig-badb9b5fa42b5250` (signal: 11, SIGSEGV: invalid memory reference)
@iwillspeak
Copy link
Collaborator

Thanks for the report. I’ll see if I can reproduce.

@iwillspeak iwillspeak self-assigned this Nov 1, 2020
@iwillspeak
Copy link
Collaborator

@plugwash Can you provide more information about the environments you're running this on? I'm not sure I have easy access to an i386 or armhf environment. Was there a particular container or VM image that you ran these tests on?

@iwillspeak
Copy link
Collaborator

Could you try a build again from the main branch? You'll need to make sure submodules are checked out for the embedded copy of libonig to be there. e.g.:

 $ git clone --recurse-submodules https://github.com/rust-onig/rust-onig

Or if you have already cloned then run

$ git submodule update --init

It would be interesting if main works as there was a recent change that's not been released yet to do with struct layout. Apparently the currently released code could be triggering undefined behavior.

@iwillspeak
Copy link
Collaborator

I suspect this has something to do with either our bindings to libonig not having the right
layouts for some things in the headers, or the way we're compiling libonig not quite being
correct on 32 bit systems.

On a 32 bit Ubuntu VM I'm seeing the following when trying to compile:

   Compiling onig_sys v69.5.1 (/vagrant/onig_sys)
warning: oniguruma/src/regparse.c: In function ‘onig_st_lookup_strend’:
warning: oniguruma/src/regparse.c:528:32: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
warning:    return onig_st_lookup(table, (st_data_t )(&key), value);
warning:                                 ^
warning: oniguruma/src/regparse.c: In function ‘onig_st_insert_strend’:
warning: oniguruma/src/regparse.c:543:34: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
warning:    result = onig_st_insert(table, (st_data_t )key, value);
warning:                                   ^
warning: oniguruma/src/regparse.c: In function ‘onig_st_lookup_callout_name_table’:
warning: oniguruma/src/regparse.c:624:32: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
warning:    return onig_st_lookup(table, (st_data_t )(&key), value);
warning:                                 ^
warning: oniguruma/src/regparse.c: In function ‘st_insert_callout_name_table’:
warning: oniguruma/src/regparse.c:644:34: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
warning:    result = onig_st_insert(table, (st_data_t )key, value);
warning:                                   ^
warning: oniguruma/src/regparse.c: In function ‘onig_foreach_name’:
warning: oniguruma/src/regparse.c:797:33: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
warning:      onig_st_foreach(t, i_names, (HashDataType )&narg);
warning:                                  ^
warning: oniguruma/src/regparse.c: In function ‘onig_renumber_name_table’:
warning: oniguruma/src/regparse.c:825:41: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
warning:      onig_st_foreach(t, i_renumber_name, (HashDataType )map);
warning:                                          ^
warning: oniguruma/src/regparse.c: In function ‘name_add’:
warning: oniguruma/src/regparse.c:1007:31: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
warning:                                (HashDataType )e);
warning:                                ^
warning: oniguruma/src/regparse.c: In function ‘callout_name_entry’:
warning: oniguruma/src/regparse.c:1445:38: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
warning:                                       (HashDataType )e);
warning:                                       ^
warning: oniguruma/src/regparse.c: In function ‘setup_ext_callout_list_values’:
warning: oniguruma/src/regparse.c:1807:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
warning:                      (st_data_t )ext);
warning:                      ^
warning: oniguruma/src/unicode.c: In function ‘onig_unicode_define_user_property’:
warning: oniguruma/src/unicode.c:1110:29: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
warning:                              (hash_data_type )((void* )e));
warning:                              ^
   Compiling onig v6.1.0 (/vagrant/onig)

Doesn't sound good.

@iwillspeak
Copy link
Collaborator

I should point out I am able to repro this crash.

test match_param::test::set_retry_limit_in_match ... error: test failed, to rerun pass '-p onig --lib'

Caused by:
  process didn't exit successfully: `/vagrant/target/debug/deps/onig-d8506b816517496c` (signal: 11, SIGSEGV: invalid memory reference)

Running under GDB it seems the issue is somewhere in the Oniguruma "string table" code
during a string comparison.

#1  0x004a3951 in onig_st_lookup (table=0xb76005e8, key=18446744072498939092, value=0xb7d78514) at oniguruma/src/st.c:253
253	  FIND_ENTRY(table, ptr, hash_val, bin_pos);

From the stack trace it looks like the key ends up as 0xffffffff, which is unfortunate.

#0  0x00483660 in str_end_cmp (x=0xb7d784d4, y=0xffffffff) at oniguruma/src/regparse.c:479
#1  0x004a3951 in onig_st_lookup (table=0xb76005e8, key=18446744072498939092, value=0xb7d78514) at oniguruma/src/st.c:253
#2  0x004837ae in onig_st_lookup_strend (table=0xb76005e8, 
    str_key=0x4eca12 "bar>o)foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", 
    end_key=0x4eca15 ">o)foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", value=0xb7d78514)
    at oniguruma/src/regparse.c:528
#3  0x00483c4b in name_find (reg=0xb7601400, 
    name=0x4eca12 "bar>o)foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", 
    name_end=0x4eca15 ">o)foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello")
    at oniguruma/src/regparse.c:756
#4  0x00483efb in name_add (reg=0xb7601400, 
    name=0x4eca12 "bar>o)foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", 
    name_end=0x4eca15 ">o)foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", backref=3, env=0xb7d78918)
    at oniguruma/src/regparse.c:991
#5  0x004913ff in parse_bag (np=0xb7d78760, tok=0xb7d78830, term=15, src=0xb7d788ac, 
    end=0x4eca18 "foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", env=0xb7d78918)
    at oniguruma/src/regparse.c:7516
#6  0x004932f6 in parse_exp (np=0xb7d78760, tok=0xb7d78830, term=0, src=0xb7d788ac, 
    end=0x4eca18 "foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", env=0xb7d78918, group_head=0)
    at oniguruma/src/regparse.c:8314
#7  0x00494257 in parse_branch (top=0xb7d787cc, tok=0xb7d78830, term=0, src=0xb7d788ac, 
    end=0x4eca18 "foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", env=0xb7d78918, group_head=0)
    at oniguruma/src/regparse.c:8715
#8  0x004943c4 in parse_alts (top=0xb7d78904, tok=0xb7d78830, term=0, src=0xb7d788ac, 
    end=0x4eca18 "foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", env=0xb7d78918, group_head=0)
    at oniguruma/src/regparse.c:8750
#9  0x00494628 in parse_regexp (top=0xb7d78904, src=0xb7d788ac, 
    end=0x4eca18 "foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", env=0xb7d78918)
    at oniguruma/src/regparse.c:8811
#10 0x004947c3 in onig_parse_tree (root=0xb7d78904, 
    pattern=0x4ec9fb "(?<foo>he)(?<bar>l+)(?<bar>o)foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", 
    end=0x4eca18 "foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", reg=0xb7601400, env=0xb7d78918)
    at oniguruma/src/regparse.c:8866
#11 0x004a22d5 in onig_compile (reg=0xb7601400, 
    pattern=0x4ec9fb "(?<foo>he)(?<bar>l+)(?<bar>o)foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", 
    pattern_end=0x4eca18 "foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", einfo=0xb7d78a78)
    at oniguruma/src/regcomp.c:7244
#12 0x004a2ae8 in onig_new (reg=0xb7d78a74, 
    pattern=0x4ec9fb "(?<foo>he)(?<bar>l+)(?<bar>o)foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", 
    pattern_end=0x4eca18 "foobarassertion failed: copyright.len() > 0onig/src/utils.rse(l+)|(r+)hello", option=0, 
    enc=0x58a940 <OnigEncodingUTF8>, syntax=0x57f1a0 <OnigSyntaxOniguruma>, einfo=0xb7d78a78) at oniguruma/src/regcomp.c:7516
#13 0x0040d686 in onig::Regex::with_options_and_encoding::h0ce8c261b5f4ace0 (pattern=..., option=..., 
    syntax=0x57f1a0 <OnigSyntaxOniguruma>) at onig/src/lib.rs:355
#14 0x0040d475 in onig::Regex::with_encoding::h4ee39f63bd7f06e4 (pattern=...) at onig/src/lib.rs:266
#15 0x0040f78e in onig::Regex::new::h4d96795137268044 (pattern=...) at onig/src/lib.rs:240
#16 0x0042cf35 in onig::names::tests::test_regex_names::h455e4262fcefd758 () at onig/src/names.rs:80
#17 0x00429432 in onig::names::tests::test_regex_names::_$u7b$$u7b$closure$u7d$$u7d$::h68058423ef1f5c1f () at onig/src/names.rs:72
#18 0x00434493 in core::ops::function::FnOnce::call_once::hcd417610f7148d8f ()
    at /rustc/18bf6b4f01a6feaf7259ba7cdae58031af1b7b39/library/core/src/ops/function.rs:227

This also appears to still be an issue on the v6.9.6_rc4 tag.

@iwillspeak
Copy link
Collaborator

Provisional fix for this in #156. If you could check that out and report back it would be much appreciated.

@plugwash
Copy link
Author

plugwash commented Nov 1, 2020

Testing with a clone of https://github.com/rust-onig and fetching the submodule failed as expected with the master branch and passed with the 32-bit-segfaults branch. I've backported the commit to the Debian package and am now running tests in that environment. The Debian autopkgtest seems to take a lot longer than plain cargo test and produce crazy ammounts of output, so i'm still waiting for it to finish, it's got way past the point where it failed before though.

@iwillspeak
Copy link
Collaborator

Nice. If it all looks good I'll pull together a new point release of onig and onig_sys for you.

@plugwash
Copy link
Author

plugwash commented Nov 2, 2020

I have uploaded the patched rust-onig-sys to debian and tests for rust-onig 5.0.0 are now passing on amd64, i386 and armhf. ppc64el is still waiting for test results (but if that does fail it will presumably be a totally separate bug) . arm64 does not seem to be getting tested for some reason.

I probably won't update rust-onig and it's reverse dependencies rust-sytect and rust-bat to the new major versions myself, i'll leave the decision on if and when to do that up to others.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants