From dc3898653e372cdf8d11e2ddff342eb0b01f04ad Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Thu, 12 Dec 2024 14:00:06 +1100 Subject: [PATCH 1/3] add API to test that libperl and the current executable are compatible Issue #22125 detected that we weren't linking the correct library with the embedded test with gcc on OpenBSD, so add an API to perform a sanity check by comparing the size of the perl interpreter structure (or its size if it was a structure) and expected perl API version between those seen in the binary and those compiled into libperl. --- embed.fnc | 4 ++++ embed.h | 1 + lib/ExtUtils/t/Embed.t | 4 +++- perl.h | 8 ++++++++ pod/perldiag.pod | 5 +++++ proto.h | 5 +++++ util.c | 46 ++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 72 insertions(+), 1 deletion(-) diff --git a/embed.fnc b/embed.fnc index 762f47f06c63..6087f3fa1ac6 100644 --- a/embed.fnc +++ b/embed.fnc @@ -663,6 +663,10 @@ Adp |SV * |amagic_deref_call \ p |bool |amagic_is_enabled \ |int method +CTdp |void |api_version_assert \ + |size_t interp_size \ + |NULLOK void *v_my_perl \ + |NN const char *api_version ETXip |void |append_utf8_from_native_byte \ |const U8 byte \ |NN U8 **dest diff --git a/embed.h b/embed.h index 6cb789ce5e02..bb93d157939a 100644 --- a/embed.h +++ b/embed.h @@ -128,6 +128,7 @@ # define _to_utf8_upper_flags(a,b,c,d,e) Perl__to_utf8_upper_flags(aTHX_ a,b,c,d,e) # define amagic_call(a,b,c,d) Perl_amagic_call(aTHX_ a,b,c,d) # define amagic_deref_call(a,b) Perl_amagic_deref_call(aTHX_ a,b) +# define api_version_assert Perl_api_version_assert # define apply_attrs_string(a,b,c,d) Perl_apply_attrs_string(aTHX_ a,b,c,d) # define apply_builtin_cv_attributes(a,b) Perl_apply_builtin_cv_attributes(aTHX_ a,b) # define atfork_lock Perl_atfork_lock diff --git a/lib/ExtUtils/t/Embed.t b/lib/ExtUtils/t/Embed.t index 1057f8179096..66353ae4ca36 100644 --- a/lib/ExtUtils/t/Embed.t +++ b/lib/ExtUtils/t/Embed.t @@ -158,7 +158,7 @@ $embed_test = "run/nodebug $exe" if $^O eq 'VMS'; print "# embed_test = $embed_test\n"; $status = system($embed_test); print (($status? 'not ':'')."ok 10 # system returned $status\n"); -unlink($exe,"embed_test.c",$obj); +#unlink($exe,"embed_test.c",$obj); unlink("$exe.manifest") if $cl and $Config{'ccversion'} =~ /^(\d+)/ and $1 >= 14; unlink("$exe$Config{exe_ext}") if $skip_exe; unlink("embed_test.map","embed_test.lis") if $^O eq 'VMS'; @@ -196,6 +196,8 @@ int main(int argc, char **argv, char **env) { perl_construct(my_perl); PL_exit_flags |= PERL_EXIT_WARN; + PERL_API_VERSION_CHECK; + my_puts("ok 3"); perl_parse(my_perl, NULL, (sizeof(cmds)/sizeof(char *))-1, (char **)cmds, env); diff --git a/perl.h b/perl.h index 5aaeea0cc223..3a90a9ca965d 100644 --- a/perl.h +++ b/perl.h @@ -9264,6 +9264,14 @@ END_EXTERN_C # define PERL_STACK_REALIGN #endif +#ifdef MULTIPLICITY +# define PERL_API_VERSION_ASSERT \ + Perl_api_version_assert(sizeof(PerlInterpreter), aTHX, PERL_API_VERSION_STRING) +#else +# define PERL_API_VERSION_ASSERT \ + Perl_api_version_assert(sizeof(PerlInterpreter), NULL, PERL_API_VERSION_STRING) +#endif + /* (KEEP THIS LAST IN perl.h!) diff --git a/pod/perldiag.pod b/pod/perldiag.pod index 69e332ded180..842b3c0906a6 100644 --- a/pod/perldiag.pod +++ b/pod/perldiag.pod @@ -4020,6 +4020,11 @@ See L. by a missing delimiter on a string or pattern, because it eventually ended earlier on the current line. +=item Mismatch between expected and libperl %s + +(F) For an embedded perl, the perl headers and configuration you built +your binary against don't match the library you've linked with. + =item Mismatched brackets in template (F) A pack template could not be parsed because pairs of C<[...]> or diff --git a/proto.h b/proto.h index d648766b4898..f11f4ca79a8b 100644 --- a/proto.h +++ b/proto.h @@ -193,6 +193,11 @@ Perl_amagic_is_enabled(pTHX_ int method) __attribute__visibility__("hidden"); #define PERL_ARGS_ASSERT_AMAGIC_IS_ENABLED +PERL_CALLCONV void +Perl_api_version_assert(size_t interp_size, void *v_my_perl, const char *api_version); +#define PERL_ARGS_ASSERT_API_VERSION_ASSERT \ + assert(api_version) + PERL_CALLCONV SSize_t Perl_apply(pTHX_ I32 type, SV **mark, SV **sp) __attribute__visibility__("hidden"); diff --git a/util.c b/util.c index 069ec59c1747..30e5596d29c4 100644 --- a/util.c +++ b/util.c @@ -5737,6 +5737,52 @@ S_xs_version_bootcheck(pTHX_ SSize_t items, SSize_t ax, const char *xs_p, } } +/* +=for apidoc api_version_assert + +Used by the PERL_API_VERSION_CHECK macro to compare the perl the +object was built with and the perl that C was built with. + +This can be used to ensure that these match and produces a more +diagnosable than random crashes and mis-behaviour. + +=cut +*/ + +void +Perl_api_version_assert(size_t interp_size, void *v_my_perl, + const char *api_version) { + dTHX; + + PERL_ARGS_ASSERT_API_VERSION_ASSERT; + + if (interp_size != sizeof(PerlInterpreter)) { + /* detects various types of configuration mismatches */ + /* diag_listed_as: Mismatch between expected and libperl %s */ + Perl_croak(aTHX_ + "Mismatch between expected and libperl interpreter structure size %zd vs %zd", + interp_size, sizeof(PerlInterpreter)); + } + if ( +#ifdef MULTIPLICITY + v_my_perl != my_perl +#else + v_my_perl != NULL +#endif + ) { + /* detect threads vs non-threads mismatch */ + /* diag_listed_as: Mismatch between expected and libperl %s */ + Perl_croak(aTHX_ + "Mismatch between expected and libperl interpreter pointer"); + } + if (strNE(api_version, PERL_API_VERSION_STRING)) { + /* diag_listed_as: Mismatch between expected and libperl %s */ + Perl_croak(aTHX_ + "Mismatch between expected and libperl API versions %s vs %s", + api_version, PERL_API_VERSION_STRING); + } +} + PERL_STATIC_INLINE bool S_gv_has_usable_name(pTHX_ GV *gv) { From 42c2d2a42333a0a352e0489bfd5faabe8b1e9323 Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Thu, 12 Dec 2024 13:29:55 +1100 Subject: [PATCH 2/3] openbsd: ensure we link to the built libperl.a, not the system libperl.a When building with gcc, lib/ExtUtils/t/Embed.t would link against the system libperl.a rather than the newly built libperl.a. Due to the limited API used by the sample code this typically didn't crash, but some configuration changes could result in a crash or a link error. For OpenBSD, change the link options to more closely match those used when building the perl executable, which results in linking against the correct library. Fixes #22125 --- lib/ExtUtils/t/Embed.t | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/ExtUtils/t/Embed.t b/lib/ExtUtils/t/Embed.t index 66353ae4ca36..49eb0a90052f 100644 --- a/lib/ExtUtils/t/Embed.t +++ b/lib/ExtUtils/t/Embed.t @@ -103,13 +103,26 @@ if ($^O eq 'VMS') { elsif ($^O eq 'os390' && $Config{usedl}) { push(@cmd,"-L$lib", ldopts()); } else { # Not MSWin32 or OS/390 (z/OS) dynamic. - push(@cmd,"-L$lib",'-lperl'); + my $ldopts = ldopts(); + if ($^O eq 'openbsd' && $Config{useshrplib} eq "false") { + # see github #22125 + # with OpenBSD, the packaged gcc (tries to) link + # against the system libperl, this will be fine once + # this perl gets installed, but that's not so good when + # testing against the uninstalled perl. + # This also matches how Makefile.SH links the perl executable + push @cmd, "$lib/libperl.a"; + $ldopts =~ s/ -lperl\b//; + } + else { + push(@cmd,"-L$lib",'-lperl'); + } local $SIG{__WARN__} = sub { warn $_[0] unless $_[0] =~ /No library found for .*perl/ }; push(@cmd, '-Zlinker', '/PM:VIO') # Otherwise puts a warning to STDOUT! if $^O eq 'os2' and $Config{ldflags} =~ /(?= 14; unlink("$exe$Config{exe_ext}") if $skip_exe; unlink("embed_test.map","embed_test.lis") if $^O eq 'VMS'; @@ -196,7 +209,7 @@ int main(int argc, char **argv, char **env) { perl_construct(my_perl); PL_exit_flags |= PERL_EXIT_WARN; - PERL_API_VERSION_CHECK; + PERL_API_VERSION_ASSERT; my_puts("ok 3"); From 060adda81e33b26d0528bad8a2cac9890ceac2c0 Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Tue, 15 Apr 2025 15:06:39 +1000 Subject: [PATCH 3/3] perldelta for fix embedding test on openbsd --- pod/perldelta.pod | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pod/perldelta.pod b/pod/perldelta.pod index ad3f5719f6b0..312446d3f4cb 100644 --- a/pod/perldelta.pod +++ b/pod/perldelta.pod @@ -289,7 +289,8 @@ made: =item * -XXX +When testing embedding, add a sanity check to ensure the C we +link against matches the perl we are building. [GH #22125] =back @@ -338,9 +339,10 @@ L section. =over 4 -=item XXX-some-platform +=item OpenBSD -XXX +When testing embedding, ensure we link against the correct static +libperl. [GH #22125] =back