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

no_panic attribute applied to function that can panic (SafePatchIterator::next(), I think) #167

Closed
hrydgard opened this issue Sep 24, 2019 · 5 comments

Comments

@hrydgard
Copy link
Contributor

hrydgard commented Sep 24, 2019

Linking with tract like this:

tract-core = "0.5.0"
tract-onnx = "0.5.0"

I'm getting the following linker error on Windows, using Rust 1.37:

         lld-link: error: undefined symbol:

          ERROR[no-panic]: detected panic in function `next`

          >>> referenced by libtract_core-36f780e6812f081d.rlib(tract_core-36f780e6812f081d.tract_core.blvpe490-cgu.9.rcgu.o):(_ZN4core3ptr18real_drop_in_place17h27638d8e99f97d5cE)
          >>> referenced by libtract_core-36f780e6812f081d.rlib(tract_core-36f780e6812f081d.tract_core.blvpe490-cgu.9.rcgu.o):(_ZN165_$LT$$LT$tract_core..ops..cnn..patches..SafePatchIterator$u20$as$u20$core..iter..traits..iterator..Iterator$GT$..next..__NoPanic$u20$as$u20$core..ops..drop..Drop$GT$4drop17hdff2e4cd80e88a13E)

Seems like no_panic is used to ensure that there are no panics in SafePatchIterator::next(), but one is in fact there. I don't see the possible panic myself, but something may have changed in a new version of Rust?

Can easily clone and patch this out myself, of course, but this seems fishy.

EDIT: Confirmed this is an issue on current master, and that taking out no_panic makes it link correctly.

@kali
Copy link
Collaborator

kali commented Sep 24, 2019

Thanks for the report, Windows support is relatively recent, and the CI coverage is still weak...

Does the command line project build in release mode or does it fail at linking with this error too ?

In the meantime, I'll try to build a few more things in the windows CI.

@kali
Copy link
Collaborator

kali commented Sep 24, 2019

dtolnay/no-panic#9

@kali
Copy link
Collaborator

kali commented Sep 24, 2019

So the issue is no_panic indeed, when linking without LTO. OS seems to be neutral to this, linux builds behaved the same. I remove no_panic, it was only protecting two functions anyway, and I can still put it back temporarily if needed.

Side bonus, windows CI now check the command line build.

@kali
Copy link
Collaborator

kali commented Sep 24, 2019

Merged, released as 0.5.1

@hrydgard
Copy link
Contributor Author

Cool, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants