Skip to content

Windows Miscompliation for String Manipulation (stable, beta, & nightly) #83100

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

Closed
jhwgh1968 opened this issue Mar 14, 2021 · 4 comments
Closed
Labels
C-bug Category: This is a bug.

Comments

@jhwgh1968
Copy link

jhwgh1968 commented Mar 14, 2021

Overview

It's a giant mess! 😄 Please ping the Windows team, stat! 🚑

One part is a duplicate, the other part is my bad.

I tried this code

Cross compiling on Linux to Windows.

.cargo/config:

[target.x86_64-pc-windows-gnu]
linker = "/usr/bin/x86_64-w64-mingw32-gcc"
rustflags = [ "-C", "target-cpu=sandybridge", "-C", "panic=abort", "-C", "lto=fat", "-C", "embed-bitcode=yes"]

src/main.rs:

use anyhow;
use itertools::Itertools;
use url::Url;

enum SubForum {
    Help,
    Community,
    Announcements,
}

fn url_for_post_number(f: SubForum, id: u32) -> Result<Url, url::ParseError> {
    dbg!(id);
    let domain = "https://users.rust-lang.org";

    dbg!(&domain);

    let path: &str = match f {
        SubForum::Help => "/c/help",
        SubForum::Community => "/c/community",
        SubForum::Announcements => "/c/announcements/{}",
    };

    dbg!(path);

    let mut final_url: String = domain.to_string();
    final_url.push('/');
    final_url.push_str(path);
    final_url += &format!("/{}", id);

    dbg!(&final_url);

    // Remove any //s due to issues with the filled-in values.
    while let Some(pos) = final_url
        .chars()
        .skip(8)
        .tuple_windows()
        .position(|(a, b)| a == '/' && b == '/')
    {
        debug_assert_eq!(final_url.remove(pos + 8), '/');
    }

    dbg!(&final_url);

    Url::parse(&final_url)
}

fn main() -> anyhow::Result<()> {
    let important_topic_id = 1234;
    let important_topic_url = url_for_post_number(SubForum::Help, important_topic_id)?;
    println!("important post url: {}", important_topic_url);
    
    Ok(())
}

I expected this to happen

[src/main.rs:12] id = 1234
[src/main.rs:15] &domain = "https://users.rust-lang.org"
[src/main.rs:23] path = "/c/help"
[src/main.rs:30] &final_url = "https://users.rust-lang.org//c/help/1234"
[src/main.rs:42] &final_url = "https://users.rust-lang.org/c/help/1234"
important post url: https://users.rust-lang.org/c/help/1234

Instead, this happened

With a release build on current Beta, Stable, and Nightly it outputs:

[src/main.rs:12] id = 1234
[src/main.rs:15] &domain = "https://users.rust-lang.org"
[src/main.rs:23] path = "/c/help"
[src/main.rs:30] &final_url = "https://users.rust-lang.org//c/help/1234"

It then busy-waits infinitely. The while loop is the apparent cause, as avoiding putting any two /s together prevents the issue.

On current nightly in debug mode, it panics:

[src/main.rs:12] id = 1234
[src/main.rs:15] &domain = "thread 'main' panicked at 'index out of bounds: the len is 31 but the index is 184467440
73709551615', library\core\src\unicode\unicode_data.rs:82:62
stack backtrace:
   0: rust_begin_unwind
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\std\src\panicking.rs:493:5
   1: core::panicking::panic_fmt
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\core\src\panicking.rs:92:14
   2: core::panicking::panic_bounds_check
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\core\src\panicking.rs:69:5
   3: core::unicode::unicode_data::skip_search::{{closure}}
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\core\src\unicode\unicode_data.rs:82:62
   4: core::option::Option<T>::map
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\core\src\option.rs:487:29
   5: core::unicode::unicode_data::skip_search
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\core\src\unicode\unicode_data.rs:82:9
   6: core::unicode::unicode_data::grapheme_extend::lookup
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\core\src\unicode\unicode_data.rs:305:9
   7: core::char::methods::<impl char>::is_grapheme_extended
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\core\src\char\methods.rs:859:9
   8: core::char::methods::<impl char>::escape_debug_ext
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\core\src\char\methods.rs:415:46
   9: core::char::methods::<impl char>::escape_debug
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\core\src\char\methods.rs:461:9
  10: <str as core::fmt::Debug>::fmt
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\core\src\fmt\mod.rs:2057:23
  11: <&T as core::fmt::Debug>::fmt
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\library\core\src\fmt\mod.rs:2010:62
  12: <&T as core::fmt::Debug>::fmt
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\library\core\src\fmt\mod.rs:2010:62
  13: <&T as core::fmt::Debug>::fmt
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\library\core\src\fmt\mod.rs:2010:62
  14: core::fmt::run
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\core\src\fmt\mod.rs:1135:5
  15: core::fmt::write
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\core\src\fmt\mod.rs:1103:26
  16: std::io::Write::write_fmt
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\std\src\io\mod.rs:1567:15
  17: <&std::io::stdio::Stderr as std::io::Write>::write_fmt
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\std\src\io\stdio.rs:859:9
  18: <std::io::stdio::Stderr as std::io::Write>::write_fmt
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\std\src\io\stdio.rs:833:9
  19: std::io::stdio::print_to
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\std\src\io\stdio.rs:939:21
  20: std::io::stdio::_eprint
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\/library\std\src\io\stdio.rs:963:5
  21: windows_test::url_for_post_number
             at /tmp/windows_test/src/main.rs:15:5
  22: windows_test::main
             at /tmp/windows_test/src/main.rs:49:31
  23: core::ops::function::FnOnce::call_once
             at /rustc/b3e19a221e63dcffdef87e12eadf1f36a8b90295\library\core\src\ops\function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

On current stable and beta in debug mode, it works correctly.

Rust Versions Showing Failure

Beta: rustc 1.51.0-beta.6 (6a1835ad7 2021-03-12)
Stable: rustc 1.50.0 (cb75ad5db 2021-02-10)
Nightly: rustc 1.52.0-nightly (b3e19a221 2021-03-12)

Environment

This behavior was first observed under Wine on Linux.

I have attempted to test the release-mode binary generated by beta on Windows, but failed to do so. It prints an error suggesting Windows Defender is blocking it from running.

I consider that circumstantial evidence the bug would occur on Windows, since malware might try to "busy-wait past" the pre-execution AV engines typically do. But I cannot say for sure.

Conclusion

All I know is:

  1. It's a mess
  2. Wine has never failed to emulate breakage and bad behavior correctly for me, so I think it's real
  3. The .cargo/config is one I have used in the past for other tools

Thank you for your time.

@jhwgh1968 jhwgh1968 added the C-bug Category: This is a bug. label Mar 14, 2021
@Aaron1011
Copy link
Member

This looks like a duplicate of #82890

@jhwgh1968
Copy link
Author

Interesting, @Aaron1011.

I notice that issue doesn't mention anything but nightly, and they say that the LLVM 12 update in the culprit.

But I still observe weird behavior on stable and beta. Perhaps this is more than one bug?

@moxian
Copy link
Contributor

moxian commented Mar 14, 2021

The nightly panic is indeed a duplicate of #82890

As per the debug/release differences on stable/beta:
the difference between working code (debug) and non-working code (release) is that the working one has debug-assertions=true

The final_url.remove(pos + 8) part of your code above never executes in release (because it's inside debug_assert_eq! macro which is stripped on release), which makes final_url.(...).position(..) always return Some(_). (because final_url is never modified).

TL;DR: Not a bug; Working as intended.

@jhwgh1968
Copy link
Author

Thanks for pointing that out, @moxian. I guess I lost that in the original code, which was much more complex.

Closing as a duplicate of #82890.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

3 participants