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

rustc_llvm: Fix flattened CLI args #131805

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

aeubanks
Copy link
Contributor

Fixes string manipulation errors introduced in #130446.

Fixes string manipulation errors introduced in rust-lang#130446.
@rustbot
Copy link
Collaborator

rustbot commented Oct 16, 2024

r? @cuviper

rustbot has assigned @cuviper.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 16, 2024
@aeubanks
Copy link
Contributor Author

r? augie

@rustbot
Copy link
Collaborator

rustbot commented Oct 16, 2024

Failed to set assignee to augie: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@aeubanks
Copy link
Contributor Author

r? durin42

@rustbot rustbot assigned durin42 and unassigned cuviper Oct 16, 2024
@rust-log-analyzer

This comment has been minimized.

@workingjubilee
Copy link
Member

Thanks.

I'm... actually not sure how rustc finished bootstrap on the LLVM 20 integration build with this error? It has built since then, right?

@workingjubilee workingjubilee added the llvm-main Marks PRs that are making Rust work with LLVM main (this label is consumed by CI tooling) label Oct 16, 2024
@aeubanks
Copy link
Contributor Author

This only affects the command line portion in debug info on windows AFAIK. The only reason we found this was because we had tests for deterministic builds on windows, and the bad strings were sometimes garbage.

@workingjubilee
Copy link
Member

workingjubilee commented Oct 16, 2024

huuuh.

/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0xbb2be6)[0x7fb88cd3fbe6]
/lib/x86_64-linux-gnu/libc.so.6(+0x3c050)[0x7fb88bfe2050]
/lib/x86_64-linux-gnu/libc.so.6(+0x15b33c)[0x7fb88c10133c]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(LLVMRustCreateTargetMachine+0x494)[0x7fb88d224a54]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0x101decb)[0x7fb88d1aaecb]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0x101ca7e)[0x7fb88d1a9a7e]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0xf52efd)[0x7fb88d0dfefd]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0x1024ded)[0x7fb88d1b1ded]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0x102428f)[0x7fb88d1b128f]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0xf618d1)[0x7fb88d0ee8d1]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(_RNvXs5_Cs1wnS3Bmr5sj_18rustc_codegen_llvmNtB5_18LlvmCodegenBackendNtNtNtCsly6bReQAbem_17rustc_codegen_ssa6traits7backend14CodegenBackend13codegen_crate+0xca)[0x7fb88d0ce5ba]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0xee55d7)[0x7fb88d0725d7]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0xe5c2e0)[0x7fb88cfe92e0]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(_RNvMs3_NtCslONYkxYabyj_15rustc_interface7queriesNtB5_6Linker24codegen_and_build_linker+0x22)[0x7fb88d094de2]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0xbbb2b5)[0x7fb88cd482b5]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0xbb37a5)[0x7fb88cd407a5]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0xb81293)[0x7fb88cd0e293]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0xbc4a7b)[0x7fb88cd51a7b]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0xbbcc82)[0x7fb88cd49c82]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0xb7cfd7)[0x7fb88cd09fd7]
/var/lib/buildkite-agent/builds/rust-llvm-integrate/llvm-project/rust-llvm-integrate-prototype/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-ad72a12415e858c4.so(+0x6c6f7db)[0x7fb892dfc7db]
/lib/x86_64-linux-gnu/libc.so.6(+0x89134)[0x7fb88c02f134]
/lib/x86_64-linux-gnu/libc.so.6(__clone+0x40)[0x7fb88c0aea40]
note: we would appreciate a report at https://github.com/rust-lang/rust
help: you can increase rustc's stack size by setting RUST_MIN_STACK=16777216

Comment on lines +493 to +499
auto ArgsCppStr = std::string(ArgsCstrBuff + buffer_offset,
ArgsCstrBuffLen - buffer_offset);
auto i = 0;
while (i != std::string::npos) {
i = ArgsCppStr.find('\0', i + 1);
if (i != std::string::npos)
ArgsCppStr.replace(i, i + 1, " ");
ArgsCppStr.replace(i, 1, " ");
Copy link
Member

Choose a reason for hiding this comment

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

So, re-reading the reference about std::string::npos, I think I may have misunderstood something about it.

Why is it correct at all as part of a conditional expression like a while or if? It only takes on its magic "until the end of the string" meaning when used as an argument to std::string's functions, doesn't it? Which the control-flow expressions are not invocations of.

Copy link

Choose a reason for hiding this comment

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

https://cplusplus.com/reference/string/string/find/

Return Value
The position of the first character of the first match.
If no matches were found, the function returns string::npos.

npos isn't magic, it's in practice just a way to say -1. find() returns npos if something isn't found, and at that point we don't take this if / exit the loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm-main Marks PRs that are making Rust work with LLVM main (this label is consumed by CI tooling) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants