Skip to content

pp.h: Use STATIC_ASSERT_EXPR on SETs#24587

Open
khwilliamson wants to merge 1 commit into
Perl:bleadfrom
khwilliamson:STATIC_ASSERT_SETs
Open

pp.h: Use STATIC_ASSERT_EXPR on SETs#24587
khwilliamson wants to merge 1 commit into
Perl:bleadfrom
khwilliamson:STATIC_ASSERT_SETs

Conversation

@khwilliamson

Copy link
Copy Markdown
Contributor

This makes sure that the argument whose pointer is being passed has the correct size to be settable by the called element.

  • This set of changes does not require a perldelta entry.

This makes sure that the argument whose pointer is being passed has the
correct size to be settable by the called element.
Comment thread pp.h
#define mXPUSHu(u) STMT_START { EXTEND(sp,1); mPUSHu(u); } STMT_END

#define SETs(s) (*sp = s)
#define SETs(s) (STATIC_ASSERT_EXPR(sizeof(*sp) == sizeof(s)), *sp = s)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When wouldn't this be caught by the compiler?

This could only be a problem when called from non-XS code (ie. no dSP explicit or implicit), since *SP is a SV*, s can only be a pointer (always the same size as SV* on anything we use) or an integer constant that evaluates to zero, which is valid but rare (same as NULL).

Non-SV pointers (which this change doesn't catch) give you:

<source>:11:5: warning: incompatible pointer types assigning to 'SV *' (aka 'struct sv *') from 'int *' [-Wincompatible-pointer-types]
   11 |     SETs(&x);
      |     ^    ~~
/opt/compiler-explorer/perl-5.42.2/lib/5.42.2/x86_64-linux-thread-multi/CORE/pp.h:583:23: note: expanded from macro 'SETs'
  583 | #define SETs(s)         (*sp = s)
      |                              ^ ~
1

Non pointers/non-integer:

<source>:11:5: error: assigning to 'SV *' (aka 'struct sv *') from incompatible type 'double'
   11 |     SETs(0.0);
      |     ^    ~~~
/opt/compiler-explorer/perl-5.42.2/lib/5.42.2/x86_64-linux-thread-multi/CORE/pp.h:583:23: note: expanded from macro 'SETs'
  583 | #define SETs(s)         (*sp = s)
      |                              ^ ~

clang complains about this valid but strange value (same as SETs(0)):

<source>:12:10: warning: expression which evaluates to zero treated as a null pointer constant of type 'SV *' (aka 'struct sv *') [-Wnon-literal-null-conversion]
   12 |     SETs('\0');
      |          ^~~~
/opt/compiler-explorer/perl-5.42.2/lib/5.42.2/x86_64-linux-thread-multi/CORE/pp.h:583:25: note: expanded from macro 'SETs'
  583 | #define SETs(s)         (*sp = s)
      |                                ^

(clang++/g++ report an error, since '\0' is a char in C++ and an int in C)

Tested here.

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

Successfully merging this pull request may close these issues.

2 participants