`proc_macro` that fails to link causes difficult-to-diagnose errors

mmastrac commented 1 year ago

Code

This is a particularly challenging one to reproduce as it requires a number of prerequisites:

A library LIB that doesn't link necessary symbols in a DOCS_RS=1 build (for performance reasons)
A proc_macro crate PROC that depends on that library LIB
Another library that imports that PROC proc_macro and attempts to compile in DOCS_RS=1.

No code provided, however we have a docs.rs run where this is visible:

https://docs.rs/crate/deno_core/0.197.0/builds/870081

Current output

[INFO] [stderr] warning: Rustdoc did not scrape the following examples because they require dev-dependencies: http_bench_json_ops, panik, fs_module_loader, hello_world, schedule_task, ts_module_loader, disable_ops, eval_js_value, wasm
[INFO] [stderr]     If you want Rustdoc to scrape these examples, then add `doc-scrape-examples = true`
[INFO] [stderr]     to the [[example]] target configuration of at least one example.
[INFO] [stderr] warning: Target filter specified, but no targets matched. This is a no-op
[INFO] [stderr]  Documenting deno_core v0.197.0 (/opt/rustwide/workdir)
[INFO] [stderr] error[E0463]: can't find crate for `deno_ops`
[INFO] [stderr]   --> lib.rs:44:9
[INFO] [stderr]    |
[INFO] [stderr] 44 | pub use deno_ops::op;
[INFO] [stderr]    |         ^^^^^^^^ can't find crate
[INFO] [stderr]

Desired output

error[XXX]: `proc_macro` crate failed to link
undefined symbol: v8__String__kMaxLength

Rationale and extra context

Originally reported as docs.rs bug https://github.com/rust-lang/docs.rs/issues/2178, but it turns out that this is indeed a legit bug in deno_ops/rusty_v8.

One big challenge to diagnosing this is determining that this is actually a link/dynamic load failure in a proc macro. The proc macro itself documents itself perfectly fine on docs.rs, but because of an optimization in rusty_v8 (https://github.com/denoland/rusty_v8/blob/main/build.rs#L52), attempting to document a crate that uses the proc macro will blow up because that proc macro is unable to link the code it needs to run.

This is not a bug in docs.rs nor rustc per-se, but it is quite difficult to determine what failed in the build process here and some additional diagnostics could be useful.

@Nemo157 helped identify this, but needed to enable some debug logging for rustc to diagnose it.

Other cases

No response

Anything else?

No response

bjorn3 commented 1 year ago

Looks like at least at https://github.com/rust-lang/rust/blob/c115ec11d2087050dc12c5c83959979aa98bb3e5/compiler/rustc_metadata/src/creader.rs#L1045 the Dlsym error is discarded.

Nemo157 commented 1 year ago

Minimal example:

# Cargo.toml
[package]
name = "foo"
version = "0.1.0"
edition = "2021"
[lib]
proc-macro=true

// src/lib.rs
extern {
    fn extern_fn();
}

/// ```
/// #[foo::bar] const _: () = ();
/// ```
#[proc_macro]
pub fn bar(tokens: proc_macro::TokenStream) -> proc_macro::TokenStream {
    unsafe { extern_fn() };
    tokens
}

// tests/test.rs
#[foo::bar] const _: () = ();

This shows that there's a difference whether you're using extern crate (as implicitly done by doc-tests) or not:

> cargo test --tests
...
error[E0463]: can't find crate for `foo`
 --> tests/test.rs:1:3
  |
1 | #[foo::bar] const _: () = ();
  |   ^^^ can't find crate
...

> cargo test --doc
...
error: /home/nemo157/.cargo/shared-target/debug/deps/libfoo-13976dd032b80415.so: undefined symbol: extern_fn
 --> src/lib.rs:5:1
  |
3 | extern crate r#foo;
  | ^^^^^^^^^^^^^^^^^^^
...

torshepherd commented 1 month ago

Bumping this issue.

We see this in rules_rust with custom cc_toolchains that specify custom dynamic linker paths. What happens is that proc-macros compiled for Host are dynamically linked to some custom GLIBC that requires a higher version than found by default. Then, when a crate depends on that proc macro, rustc fails to dlsym the proc macro crate but the only error it prints out is "can't find crate time_macros" (for instance)

It would be great if we could surface the dlsym error as a compilation error instead of silently failing, then users could debug whether it's a GLIBC issue or a missing symbol issue or whatever it may be

rust-lang / rust