The different xs_handshake() failures need distinct messages

[nwc10]

xs_handshake() makes two different comparisons that on failure are reported
as "got handshake key %p, needed %p", with opaque hexadecimal values.

The first is the "actual" key as generated by the HS_KEY() macro, which
encodes various values such as sizeof(PerlInterpreter) and the API version.
The second is the address of the current thread's PerlInterpreter struct.

Either can fail, and before this commit they would fail with identical text.
Hence it wasn't obvious what the problem was, causing "confusion and delay"
if one tried to decode the hexadecimal output as the wrong thing. (For
example when it's actually pointers mismatching, but one tries to decode the
values into API version and structure size, assuming that the values were
the packed output from HS_KEY().)

[iabyn]

On Thu, Sep 09, 2021 at 03:07:46AM -0700, Nicholas Clark wrote: xs_handshake() makes two different comparisons that on failure are reported as "got handshake key %p, needed %p", with opaque hexadecimal values.

Personally I'd go further. This message most often appears when modules built with one version of perl are loaded by a different perl binary. This is a fairly common occurrence (e.g. if PERL5LIB is set), and the message is *horrible*. It's going on about a 'handshake' having the wrong hex key, and just scares the user without saying anything useful. Personally I think the error message should: * remove any mention of 'handshake' (it makes it sound like some sort of network error); * should be a lot more verbose, suggesting (where relevant) that its likely a mismatch between perl version/build and module build, and displaying (where relevant) the @inc path and relevant PERL env vars. So, a bit like how we improved the 'module not found' error: Can't locate Foo/Bar.pm in @inc (you may need to install the Foo::Bar module) ***@***.*** contains: ....). * If the hex values have any further useful info, they should be symbolically decoded if possible.

…

-- In my day, we used to edit the inodes by hand. With magnets.

[nwc10]

Personally I'd go further. ...

Agree. (with the reasoning and the detailed suggestions.)

Right now I have a TODO list of open things that I'm trying to put to bed, so I don't want to go sideways onto this. Also, I think that this commit is better than what we had, and doesn't get in the way of further improvement, so I'm going to merge it instead of putting it on hold until me (or someone else) takes up this plan.

[jkeenan]

Is there any way we can in the test suite simulate the conditions which would generate this warning?

At present, it appears to be untested.

$ ack 'loadable library and perl binaries are mismatched' .
pod/perldiag.pod
3423:=item %s: loadable library and perl binaries are mismatched (got %s handshake key %p, needed %p)

util.c
5632:            noperl_die("%s: loadable library and perl binaries are mismatched"

[nwc10]

Is there any way we can in the test suite simulate the conditions which would generate this warning?

I can't think of any easy way. We can't rely on anything outside of the build tree, and we need a shared object that will load on the CPU and OS we're building on, so we can't pre-compile test files either. So I think we'd have to build a test shared object that we knew is "wrong", and then try to load it. So we'd

1. need a specific extension just for this - sort of XS::API-Test::Handshake::Fail
2. also have to add complexity to the core's macros so that we can compile this module with some "wrong" inputs so that the "handshake" fails

which seems like a lot of work to get to pass, and likely fragile and going to break quite often.

Unless I've missed a simpler way of deliberately getting this code to hit the error path...