Repository navigation
Conversation
Signed-off-by: Amirhossein Akhlaghpour <m9.akhlaghpoor@gmail.com>
Signed-off-by: Amirhossein Akhlaghpour <m9.akhlaghpoor@gmail.com>
| "conflicting thread-local and non-thread-local declarations for `{symbol_name}`" | ||
| ), | ||
| ); | ||
| } |
There was a problem hiding this comment.
I think it would be nicer to instead have declare_data return an error that we can catch. Opened bytecodealliance/wasmtime#14469 for this.
There was a problem hiding this comment.
makes sense but as #14469 is still unmerged and we're still on cranelift-module 0.135.0 that error isn't available here yet
I can switch this to handling the declare_data error once the Cranelift change is available here
| // This test checks a cg_clif diagnostic; LLVM accepts the conflicting declarations. | ||
| if matches!(cg_clif_dylib, CodegenBackend::Builtin(name) if name == "llvm") { | ||
| skip_tests.push("aot.tls_conflicting_declarations"); | ||
| } |
There was a problem hiding this comment.
You could also disable it in .github/workflows/main.yml in the test_llvm job under "Disable JIT tests".
| "{}{}", | ||
| String::from_utf8_lossy(&output.stdout), | ||
| String::from_utf8_lossy(&output.stderr), | ||
| ); |
There was a problem hiding this comment.
Could you directly check that stderr contains/does not contain the expected output instead?
| }), | ||
| TestCase::custom("aot.tls_conflicting_declarations", &|runner| { | ||
| let variants: &[(&str, bool)] = if runner.target_compiler.target.contains("windows") { | ||
| // Matching TLS declarations currently produce a duplicate-symbol error on Windows. |
There was a problem hiding this comment.
Huh, what is going on there?
There was a problem hiding this comment.
seems windows rejects the other two cases earlier with symbol EXPORTED is already defined before we reach the TLS declaration check
can we keep just the plain conflict case on windows and all three elsewhere ?
Signed-off-by: Amirhossein Akhlaghpour <m9.akhlaghpoor@gmail.com>
bb316b2 to
4fdf1f8
Compare
2 participants
fixes #1691
declaring the same symbol as both thread-local and non-thread-local currently makes cg_clif ICE when Cranelift tries to merge the declarations.
we can check for the mismatch first and reports a compiler error instead