-
Notifications
You must be signed in to change notification settings - Fork 571
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
util.c: Perl_xs_handshake print API ver mismatch before interp mismatch #22719
base: blead
Are you sure you want to change the base?
Conversation
e3ff370
to
8706962
Compare
repushed new rev |
8706962
to
7222838
Compare
rev # 3 pushed, I made "Perl API 5.X,0 vs 5.Y.0" POD description max details, for max fool proof, considering perl has 1st time/very rare time perl users. Like a C++ dev or Java dev who edits 3 lines of P5 code every 2-5 years for personal private code reasons, armed only knowledge, sometimes YEARS of knowledge, day job career knowledge of Java or C++ or Javascript, But this smart developer has no background info on Perl 5 and will get lost trying to change 2 or 5 lines pf Perl 5 code somewhere. I personally have zero idea how to turn a .class into a ,jar, even tho I have 6 cloned .git repo of Java right now because those libraries are very useful to me for grep research for Perl, or C, or MS C++ Win32 API code I am writing. I've reversing that viewpoint situation in the pod text to account for other 1st time Perl users. I AM NOT JOKING. There are stack overflow threads, Linux User Group forums, BSD forums, and OSX forums, where developers are ripping their hair out looking for ASCII perl code that is doing
as the trigger for
and
Yes, they think its |
7222838
to
73172a6
Compare
rebased repushed, I'd like to see this committed. It also adds CI code that proves the fatal errors will execute. |
73172a6
to
e48f517
Compare
|
if(ix == 0) { | ||
#ifdef MULTIPLICITY | ||
Perl_xs_handshake(HS_KEYp(sizeof(PerlInterpreter), | ||
TRUE, NULL, FALSE, | ||
sizeof("v1.1337.0")-1, | ||
sizeof("")-1), | ||
HS_CXT, __FILE__, items, ax, | ||
"v1.1337.0"); | ||
#else | ||
Perl_xs_handshake(HS_KEYp(sizeof(struct PerlHandShakeInterpreter), | ||
FALSE, NULL, FALSE, | ||
sizeof("v1.1337.0")-1, | ||
sizeof("")-1), | ||
HS_CXT, __FILE__, items, ax, | ||
"v1.1337.0"); | ||
#endif | ||
} | ||
else if(ix == 1) { | ||
#ifdef MULTIPLICITY | ||
Perl_xs_handshake(HS_KEYp(sizeof(PerlInterpreter)+1, | ||
TRUE, NULL, FALSE, | ||
sizeof("v" PERL_API_VERSION_STRING)-1, | ||
sizeof("")-1), | ||
HS_CXT, __FILE__, items, ax, | ||
"v" PERL_API_VERSION_STRING); | ||
#else | ||
Perl_xs_handshake(HS_KEYp(sizeof(struct PerlHandShakeInterpreter)+1, | ||
FALSE, NULL, FALSE, | ||
sizeof("v" PERL_API_VERSION_STRING)-1, | ||
sizeof("")-1), | ||
HS_CXT, __FILE__, items, ax, | ||
"v" PERL_API_VERSION_STRING); | ||
#endif | ||
} | ||
else { | ||
#ifdef MULTIPLICITY | ||
Perl_xs_handshake(HS_KEYp(sizeof(PerlInterpreter)+1, | ||
TRUE, NULL, FALSE, | ||
sizeof("v1.1337.0")-1, | ||
sizeof("")-1), | ||
HS_CXT, __FILE__, items, ax, | ||
"v1.1337.0"); | ||
#else | ||
Perl_xs_handshake(HS_KEYp(sizeof(struct PerlHandShakeInterpreter)+1, | ||
FALSE, NULL, FALSE, | ||
sizeof("v1.1337.0")-1, | ||
sizeof("")-1), | ||
HS_CXT, __FILE__, items, ax, | ||
"v1.1337.0"); | ||
#endif | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a lot of duplicated code. I believe the following is equivalent, but shorter:
if(ix == 0) { | |
#ifdef MULTIPLICITY | |
Perl_xs_handshake(HS_KEYp(sizeof(PerlInterpreter), | |
TRUE, NULL, FALSE, | |
sizeof("v1.1337.0")-1, | |
sizeof("")-1), | |
HS_CXT, __FILE__, items, ax, | |
"v1.1337.0"); | |
#else | |
Perl_xs_handshake(HS_KEYp(sizeof(struct PerlHandShakeInterpreter), | |
FALSE, NULL, FALSE, | |
sizeof("v1.1337.0")-1, | |
sizeof("")-1), | |
HS_CXT, __FILE__, items, ax, | |
"v1.1337.0"); | |
#endif | |
} | |
else if(ix == 1) { | |
#ifdef MULTIPLICITY | |
Perl_xs_handshake(HS_KEYp(sizeof(PerlInterpreter)+1, | |
TRUE, NULL, FALSE, | |
sizeof("v" PERL_API_VERSION_STRING)-1, | |
sizeof("")-1), | |
HS_CXT, __FILE__, items, ax, | |
"v" PERL_API_VERSION_STRING); | |
#else | |
Perl_xs_handshake(HS_KEYp(sizeof(struct PerlHandShakeInterpreter)+1, | |
FALSE, NULL, FALSE, | |
sizeof("v" PERL_API_VERSION_STRING)-1, | |
sizeof("")-1), | |
HS_CXT, __FILE__, items, ax, | |
"v" PERL_API_VERSION_STRING); | |
#endif | |
} | |
else { | |
#ifdef MULTIPLICITY | |
Perl_xs_handshake(HS_KEYp(sizeof(PerlInterpreter)+1, | |
TRUE, NULL, FALSE, | |
sizeof("v1.1337.0")-1, | |
sizeof("")-1), | |
HS_CXT, __FILE__, items, ax, | |
"v1.1337.0"); | |
#else | |
Perl_xs_handshake(HS_KEYp(sizeof(struct PerlHandShakeInterpreter)+1, | |
FALSE, NULL, FALSE, | |
sizeof("v1.1337.0")-1, | |
sizeof("")-1), | |
HS_CXT, __FILE__, items, ax, | |
"v1.1337.0"); | |
#endif | |
} | |
size_t interp_size; | |
bool have_multiplicity; | |
#ifdef MULTIPLICITY | |
interp_size = sizeof (PerlInterpreter); | |
have_multiplicity = TRUE; | |
#else | |
interp_size = sizeof (struct PerlHandShakeInterpreter); | |
have_multiplicity = FALSE; | |
#endif | |
if (ix > 0) { | |
interp_size++; | |
} | |
const char *vstring = | |
ix == 1 | |
? "v" PERL_API_VERSION_STRING | |
: "v1.1337.0"; | |
Perl_xs_handshake(HS_KEYp(interp_size, | |
have_multiplicity, NULL, FALSE, | |
strlen(vstring), | |
sizeof("")-1), | |
HS_CXT, __FILE__, items, ax, | |
vstring); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#ifdef MULTIPLICITY
interp_size = sizeof (PerlInterpreter);
have_multiplicity = TRUE;
#else
interp_size = sizeof (struct PerlHandShakeInterpreter);
have_multiplicity = FALSE;
#endif
This obfuscates hides or abstracts what is being tested, and what the tests (all permutations) are. Performance, or maintainability, correctness (not related to a test or proving runtime behavior or retiring a ex-valid now invalid test) or optimizing or refactoring to "modern APIs" is N/A for the stuff inside ext/XS-APItest/APItest.xs
.
My 1st revision shows all possible build configs, and how the code could compile, and what it being tested. Turning things controlled on a CPP level, into C variables (.i/runtime machine code level), it can be done if needed, but messes up understanding what is a folded constant at runtime (CPP 100% of the time), and what is a shell ENV var that is fetched at runtime. size_t interp_size
could be a shell var, it could be our $XSAPItest::interp_size = 0x10f0;
.
I'd rather not touch any previously written test, since any refactoring for the sake of refactoring always runs the risk of accidentally silently loosing test coverage of whatever that test was testing.
pod/perldiag.pod
Outdated
L<Perl API version|perldiag/"Perl API version %s of %s does not match %s"> | ||
error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably doesn't render well in plain text/man format.
I'd suggest the following, which is less pretty, but unambiguously spells out the other error we're referring to:
L<Perl API version|perldiag/"Perl API version %s of %s does not match %s"> | |
error. | |
L<perldiag/"Perl API version %s of %s does not match %s">. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Nov 2024 I did render it as HTML POD which i dont do as a routine, and it was clickable, and looked correct as HTML. L<perldiag/"Perl API version %s of %s does not match %s">
might render very badly as html like [76]
or full sentence "Perl API version %s of %s does not match %s"
with underscoring, but i didnt test it. Both are bad UI.
my priority is what it will look like on perldoc.perl.org and metacpan.org. Will render a clickable link or a broken anchor that leaves the users scroll bar at the top of the page? POD being readable as source code is against the purpose of using POD. the HTML will have a clickable "Perl API version" link that fits in with the sentence and looks good. Any other text string rendering in HTML else will be bad UI.
Mice are sadly obsolete HW nowadays anyway, most users can't hover over a link to see where it goes or see the actual URL before committing to a navigate event. So labels are critical.
Making a link to perldiag
that leaves the user at the top, is rude and calling the end user a lazy bum for not pressing F3 and typing a string. I want it to look nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The texts in perldiag are also shown in the terminal when one uses «use diagnostics;» and that warning is triggered, so it should absolutely be plain text usable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about `L<"Perl API version %s of %s does not match %s"|perldiag/"Perl API version %s of %s does not match %s">?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The texts in perldiag are also shown in the terminal when one uses «use diagnostics;» and that warning is triggered, so it should absolutely be plain text usable.
Is everyone aware I am NOT edit this the error message saying?
=item %s: loadable library and perl binaries are mismatched (got %s handshake key %p, needed %p)
(P) A dynamic loading library C<.so> or C<.dll> was being loaded into the
process that was built against a different build of perl than the
said library was compiled against. Reinstalling the XS module will
likely fix this error.
Users with the %s: loadable library and perl binaries are mismatched (got %s handshake key %p, needed %p)
on their screen will still have sentence /grep word phase %s: loadable library and perl binaries are mismatched (got %s handshake key %p, needed %p)
on their screen.
Instead, I am editing
Line 5281 in 6549937
=item Perl API version %s of %s does not match %s |
=item Perl API version %s of %s does not match %s
(F) The XS module in question was compiled against a different incompatible
version of Perl than the one that has loaded the XS module.
and I, adding a link to the top error message from the 2nd lower one in the 2nd lower one.
The user reading Perl API version %s of %s does not match %s
minimally cares about any other error in the world except the one on the screen. Its personal choice if they to click the link from the Perl API version %s of %s does not match %s
description, and navigate to the to the description for %s: loadable library and perl binaries are mismatched (got %s handshake key %p, needed %p)
.
They don't have a %s: loadable library and perl binaries are mismatched (got %s handshake key %p, needed %p)
problem on their system if they are reason the desc body for Perl API version %s of %s does not match %s
. No UI is lost, no sentences on either error description lost.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about `L<"Perl API version %s of %s does not match %s"|perldiag/"Perl API version %s of %s does not match %s">?
I'll change it to this even though I disagree since its a wall of text on perldoc/metacpan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repushed new version with this change
likely fix this error. This error is a less commonly seen subset of
-L<Perl API version|perldiag/"Perl API version %s of %s does not match %s">
+L<"Perl API version %s of %s does not match %s"|perldiag/"Perl API version %s of %s does not match %s">
error.
I don't think I wrote anywhere above, but the perldiag text that starts with
The term XS module does not mean a C<.pm> file. This error is not directly
caused by Perl code inside a particular C<.pm> file or C<.pl> file.
Instead this error is only caused by OS and CPU specific "shared library"
files created by a C or C++ compiler.
I wrote very specifically so a full time career person with a job title of sysadmin, CSO, devops, full stack web developer aka full time DOM/ES6 dev, or a python dev can fix the problem themselves with no outside help. All of the 6 employee types I just listed are highly intelligent people, but they only look at, read, edit, google up, perl source code once every 2-3 years. They understand Perl src code and the Perl interp is 99% identical to JS/ES5/PHP/Python, just like all French/Italian people can 2-way read each others source code in an emergency. This isn't French vs Turkish or French vs Vietnamese (x86 vs ARM or x86 vs MIPS assembly).
I can see my Python writing SO never figuring out there is machine code/C code executing in her script/app, even though she knows how to set Py lang break points. AFAIK NodeJS ecosystem doesn't have any analog of the cPython API or Perl XS. 100% of low level C-style NodeJS code is shipped as JS src code, since the JIT system/faux-C compiler linker
can't be disabled or separated from Node/V8 interp. From what I've seen, Node people will just write explicit CPU arch specific intrinsics in JS/ES lang instead of writing in C or assembler if the really need to.
Komodo GUI P5 lang debugger, and therefore P5P's perl5db.pl, single stepping gives ZERO end user UI clues that the Perl XS API and XSUBs even exist. I thought when I was very young XSUBs were actually tie
TIESCALAR
or overload.pm
PP methods that asked the P5P PP DB::* API, for themselves to be unsteppable/atomic.
Therefore I added the text the end user needs to chase down a .so/.dll file. I've seen linux text editors/psudo-POSIX IDEs/linux file pickers do really UI unfriendly or bizzare things when told to "open a .so/.elf". Worst one is a Linux text editor showing a 0 byte empty text file for a .so/.elf, and letting the user instantly truncate/overwrite their .elf/.so to a low 7 bit printable ascii file.
Obviously any Linux user dumb enough to open a .elf
in a text edit wants to convert that .elf
to a proper #!/bin/bash\n
applicatopn am I right? [NO].
So you gotta explain the basics so nobody needs to write a forum/social media post anywhere. Dentists can't organ transplants even though the government legally allows them to do so (USA, in most states).
PS, a USA dentist's commercial non-government-regulated and not government required malpractice insurance is instantly voided if do an organ transplant. Nothing stops a USA dentist from deciding to switch specialties to internal organ surgeon if they hate looking at teeth, They just need to convince an insurance provider with recent classroom training and on-job shadowing/internship to get their coverage reissued. I have to explain to a Dentist how to treat diabetes inside perldiag.pod, I wrote enough that there are no questions to ask.
This comment/post is for archive reasons for someone to read in the future.
My original CvHSCXT() macro and its implementation was poorly thought out by me, but there are too many issues/potential problems with touching it, some ABI compat concerns about dispatching good STDERR strings during the fatal exit, other concerns since I have near-future ideas or PRs about major surgery on the non-public non-CPAN XS internals of the CV body struct when the CV body struct is an XSUB and not a PP sub.
|
e48f517
to
f077dc5
Compare
repushed, new revision has 90% of the mauke fixes, last 10% of the mauke fixes i disagreed with aren't incorporated and my replies/responses to those particular lines r are above, the 2 disagreements are about writing POD to be readable in vim or readable on perldoc.perl.org, and refactoring and optimizing C code of an existing XSAPItest.xs test which I see multiple problems doing or trying to do that |
-this fatal error is much more common by general users than I (orig author) anticipated when I added this check in 5.21.6/2014. I assumed Unix land never had ABI/SEGVing or upgrade problems previous. I wrote the code for my dev style, and my personal setup as test cases, and test cases with Win32-isms. If other OSes get bad-ABI caught, its a plus, but I thought they wouldn't. -the hexadecimal handshake keys were intended to be a debug tool for core devs hacking on something and for XS authors with very complicated Makefile.PL s. To catch -D CCFLAGS arg dropouts on the way to the final cmd line invocation of the CC. -I say the handshake keys are a terrible UI for general "power users" and non-coder sys admins -the Perl API version strings ARE available, even with mismatched interp struct sizes, and those are much more user friendly to print as a error. It should be obvious that from now on, non-power users can figure out on their own (no community help) that a way to "fix" XS boot handshake is to force "reinstall" the "left side perl" or "right side perl" through the OS Pkg Manager. -after this commit, much more often! but not always, users will see a "Perl API 5.X.Y against 5.X+1.Y is incompatible" fatal message instead of the those Core-dev only undocumented hex handshake keys. Sadly the technical P5P debug info is now gone/lost/hidden if "Perl API 5.X.Y against 5.X+1.Y is incompatible" fatal message executes. -core devs, obv will have v5.X.Y matching v5.X.Y in blead perl, so they will still get the handshake keys hex numbers. Since API strings are same. -Package name will get downgraded to "Foo.c" if interp size is wrong, or 2 libperls in 1 proc happens. But the major improvement is showing left and right side Perl API version info. -The POD text is very wordy and detailed, since it has been observed over time, some Perl users, do not know Perl's backend implementation is written in the ISO C language. Or other Perl users on various internet forums or social media, do not know what the term XS code is. While they can sucessful write and debug private personal Perl 5 code, they only read the POD of CPAN modules and only use public documented APIs of CPAN modules, and rarely or never look at "private source" of CPAN modules. Therefore this group of users truly do not know MANY MANY p5p core modules and CPAN modules, make call outs to another language (C), and are unable to troubleshoot a .so file on their filing system is responsible for the error. Since they do not know about XS code concept, their troubleshooting goes very wrong as they keeping looking and keep incorrect diagnosing the problem to ASCII text somewhere in the Perl ecosystem. Either Perl source code, they wrote, or CPAN Perl source code, or a CSV, YAML or JSON file related to Perl. Make it clear, that some "Perl 5 modules" written in ASCII text in Perl 5 lang, depend on "foreign" C code and "foreign" .so/.dll files at runtime. First time Perl coders, can mistakenly assume the Perl 5 interpreter has JIT and self bootstraps to OS binaries from only Perl 5 source like Google Chrome V8 and Raku. So they guess, as first time users, Perl 5 also does it and has no dependency on legacy technologies like C or C++. This commit was specifically written for Perl#16654 but there are dozens or 100s of them Perl#19112
f077dc5
to
62a6e9f
Compare
see commit text
#19112
#16654
#15861
are most related to this patch, consideration and study should be done, if this patch, is "enough" to close those 3 tickets, and bury the handshake message drama forever, or
Dynaloader::
/strace/perldocs need improvement on how to un-brick your perl install when "Cwd.so" became corrupt.I don't have an opinion on closing those tickets.
This patch shows API version conflict error FIRST, if the API version is actually conflicted and would have executed/fataled much later if it werent stoped by the ABI checks fatal erroring themselves.
This patch swaps the struct size message and 2 libperl.so es in 1 proc message, with the API version string NEQ message, if the API version is NEQ.