rustdoc will execute unreachable macros

[bf]

I tried this code:

fn private_no_docs() { 
  let nobody_ever_use_this = include_str!("/etc/passwd-FILE_DOES_NOT_EXIST");
}

fn main() {}

I expected to see this happen: Nothing, because rustdoc is just taking the documentation comments and creates a HTML document from it.

Instead, this happened:

The include_str! macro is executed by rustdoc even though it is in unreachable code.
The macro tries to load a non-existing file and compilation stops.
rustdoc would be expected to not run this macro.

This also happens with cargo doc

 $ cargo doc
 Documenting evildependency v0.1.0 (./cargo-rustdoc/evildependency)
    Checking evildependency v0.1.0 (./cargo-rustdoc/evildependency)
error: couldn't read `/etc/passwd-FILE_DOES_NOT_EXIST`: No such file or directory (os error 2)
 --> ./cargo-rustdoc/evildependency/src/lib.rs:8:31
  |
8 |   let nobody_ever_uses_this = include_str!("/etc/passwd-FILE_DOES_NOT_EXIST");
  |                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: this error originates in the macro `include_str` (in Nightly builds, run with -Z macro-backtrace for more info)

error: could not document `evildependency`
warning: build failed, waiting for other jobs to finish...
error: could not compile `evildependency` (lib) due to 1 previous error

Meta

rustc --version --verbose:

rustc 1.83.0 (90b35a623 2024-11-26) (Arch Linux rust 1:1.83.0-1)
binary: rustc
commit-hash: 90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf
commit-date: 2024-11-26
host: x86_64-unknown-linux-gnu
release: 1.83.0
LLVM version: 18.1.8

[jieyouxu]

Triage: this is not rustdoc, this is rustc. EDIT: I see what you mean, it's both because rustdoc uses some parts of rustc. Tagging as C-discussion, T-rustdoc can mark this as C-bug if they consider this something they want to handle.

cc @rust-lang/security

[Noratrieb]

rustdoc needs to expand all macros to be able to generate documentation properly. for example, this macro could expand to an impl block, which would need to be documented.

[bf]

The function is private, I thought rustdoc will not document private functions? How can impl block break out of the scope?

[bjorn3]

The function is private, I thought rustdoc will not document private functions?

Macro expansion and parsing happen interleaved by rustc way before rustdoc gets involved. We even need to typecheck private functions.

How can impl block break out of the scope?

pub trait Foo {}
pub struct Bar;

fn baz() {
    impl Foo for Bar {}
}

will allow you to use the Foo impl for Bar outside of baz and also publicly document this impl.

[bf]

@bjorn3 your understanding of include_str() is wrong. It returns a String. The minimal example given would expand to something like let nobody_ever_use_this = "impl Foo for Bar {};".

fn private_no_docs() { 
  let nobody_ever_use_this = "impl Foo for Bar {}";
}

fn main() {}

There is no universe in which contents of a string suddenly will turn into something when you typecheck it.

You keep talking about rustc like I'm compiling the code. I want to generate documentation with rustdoc. If it calls a function from rustc in the background to understand the code, that's your implementation decision. But if your function suddenly starts running code instead of taking documentation from it that's a bug.

And it feels like the logic is all messed up: How would the syntax parser get the idea to further parse a include_str!() -> &str macro?

[bjorn3]

Consider the following:

const fn private_no_docs() -> &'static str { 
  include_str!("/etc/passwd-FILE_DOES_NOT_EXIST")
}

pub fn bar() -> [u8; private_no_docs().len()] { todo!() }

The type signature of bar shown by rustdoc will depend on the length of /etc/passwd-FILE_DOES_NOT_EXIST.

[bf]

In the bug report the function private_no_docs() IS NOT CALLED - IT IS DEAD CODE.
In your example, you call the function private_no_docs() via private_no_docs().len(), which is totally different.

[Noratrieb]

during the macro expansion stage, we do not know what is called and what isn't.

[bjorn3]

We only whether it is called or not after macro expansion at which point we can no longer go back and expand it if it turns out to be necessary after all. Knowing if a function is called or not depends on type checking, which itself depends on all trait impls being exposed which depends on all macros that could even potentially generate a trait impl being expanded. We could hard code that include_str!() doesn't generate trait impls, but that doesn't help with proc macros and is very fragile in general.

[bf]

To be honest, I only understand half the words you are saying but for me this is a bug in the way macros are processed.
The AST parser should see that private_no_docs() is unreachable. The macro parser should never touch that spot.

[bjorn3]

Let's say you have the following code:

trait A { const S: &'static str; }
struct B;

fn private_no_docs() { 
  macro_rules! foo {
    () => { impl A for B { const S: &'static str = include_str!("/etc/passwd-FILE_DOES_NOT_EXIST"); } }
  }
  foo!();
}

pub fn bar() -> [u8; <B as A>::S.len()] { todo!() }

Getting the function signature of bar requires expanding foo!(), but we don't know that this is the case until we have already expanded foo!() and all other macros.

[Noratrieb]

I can't say much else to this other than no, this is not possible. For example method lookup depends on the type system, and to have that you need all macros expanded. It's just how the compiler works and has to work. If you're interested in knowing why, I encourage reading the rustc dev guide and compiler source code to understand these words :)

[aDotInTheVoid]

closing as working-as-intended. rustdoc/rustc necessarily does total macro-expansion before it can do name resolution and then everything else.

[Manishearth]

To be honest, I only understand half the words you are saying but for me this is a bug in the way macros are processed.

This is an explicit design choice: macros get handled before anything structured is known about the code, they are pure transformations of the AST.

"Do not expand macros in unused code" sounds simple enough in theory, but it requires a far more spaghetti'd compiler architecture, and won't be able to handle edge cases like the one bjorn is talking about, at which point you have a feature which only sometimes works.

In general making AST interpretation dependent on knowing types and other is super fraught in compiler design, it leads to issues like Most Vexing Parse.

You're not wrong that a lot of clever tricks can be done here with sufficient work and a sufficiently rearchitected compiler to handle some such cases. It's just really not worth it, macros are defined to always expand and there's not really any actual problem that causes.

This would be easier in an interpreted language since you can compile things on demand, but as a compiled language the design space is very different.

[bf]

@Manishearth we both agree the thing should be refactored. I don't understand how things can be more spaghetti when rustfmt of the most robust language can SIGSEGV, rustdoc does arbitrary code execution, and crates.io modules can hack me from three dependencies down. I'm not asking about any "clever tricks" but basic software quality. It's your project, you're fine to close such issues to have a peaceful day. But I think your motivation is not software quality, it's political.

[workingjubilee]

Yes, you got me. You've discovered my political ideology of fixing SIGSEGVs.

[Manishearth]

(no, I do not think it should be refactored, I think the current architecture is broadly the right one when it comes to this. I was merely noting it is in theory possible, but there are other tradeoffs that make that architecture extremely undesired)