selectors::OpaqueElement::new is unsound

[GuillaumeGomez]

When building librsvg with the GCC backend, we got a warning about returning a pointer to a local variable here. The problem comes from OpaqueElement::new doing a reference to pointer conversion.

EDIT: The bug might be coming from the rust GCC backend, just opening this issue in case it is not wrong. Don't hesitate to close it if there is no issue.

[GuillaumeGomez]

It seems that the issue is in fact how this API is used rather than the API itself. Closing then and opening an issue on the relevant repository.

[sdroege]

AFAICS this here is all fine, but librsvg is probably wrong here. It's passing the &Ref<Node> into new(), and that's indeed an address to a local variable that is then stored in the OpaqueElement. That's not really a problem as it can only be used for comparison purposes AFAIU, but probably not the intended behaviour in librsvg.

For reference, the compiler warning from @GuillaumeGomez here is

libgccjit.so: warning: : function may return address of local variable [-Wreturn-local-addr]
libgccjit.so: note: : declared here

[federicomenaquintero]

Permalink for the offending source code in librsvg - I've fixed this by not returning a &Ref<...>, which is clearly ephemeral, but rather a reference to the actual element data.

Also, I couldn't find a way to trigger a bug due to this unsoundness. Librsvg's test coverage indicates that the opaque() method is called a few times, but I am not actually sure what sort of CSS would trigger this - maybe shadow hosts or some :nth-child, from briefly looking at the selectors source code?

(Maybe the existing tests that actually execute that code got lucky with respect to the unsoundness?)

I haven't actually looked at the coverage for selectors, though.

[federicomenaquintero]

Bug in librsvg where this was originally reported; merge request with fix.