Skip to content

Avoid undefined behaviour with signed integer multiplication#1296

Merged
mr-c merged 3 commits intosimd-everywhere:masterfrom
rathann:fix-signed-integer-overflow-ub
Jun 30, 2025
Merged

Avoid undefined behaviour with signed integer multiplication#1296
mr-c merged 3 commits intosimd-everywhere:masterfrom
rathann:fix-signed-integer-overflow-ub

Conversation

@rathann
Copy link
Contributor

@rathann rathann commented Jun 24, 2025

This is the minimal patch that makes tests pass with clang 20.1.x on Fedora rawhide x86_64 and aarch64.

Fixes #1295 .

@aqrit
Copy link
Contributor

aqrit commented Jun 25, 2025

Fixes #1295 .

simde_vmul_laneq_s32 was for an example.
The "many intrinsics" statement is key.

related:

Undefined behavior stemming from signed overflow seems to be everywhere.
At this time, I'd say that SIMDE should never be used without -fwrapv.
#901 (comment)

@rathann rathann force-pushed the fix-signed-integer-overflow-ub branch from 3abee75 to b21f29e Compare June 26, 2025 08:28
@rathann
Copy link
Contributor Author

rathann commented Jun 26, 2025

It looks like this is now failing when compiled as C++:

../test/arm/neon/../../../simde/arm/neon/mul_lane.h:389:32: error: use of old-style cast [-Werror,-Wold-style-cast]

What is the recommended way of keeping the code compatible with both C and C++? I'm not very good with C++.

@mr-c
Copy link
Collaborator

mr-c commented Jun 26, 2025

It looks like this is now failing when compiled as C++:

../test/arm/neon/../../../simde/arm/neon/mul_lane.h:389:32: error: use of old-style cast [-Werror,-Wold-style-cast]

What is the recommended way of keeping the code compatible with both C and C++? I'm not very good with C++.

My mistake, sorry

Try one of HEDLEY_STATIC_CAST or HEDLEY_REINTERPRET_CAST

simde/simde/hedley.h

Lines 842 to 858 in 51743e7

#if defined(HEDLEY_REINTERPRET_CAST)
# undef HEDLEY_REINTERPRET_CAST
#endif
#if defined(__cplusplus)
# define HEDLEY_REINTERPRET_CAST(T, expr) (reinterpret_cast<T>(expr))
#else
# define HEDLEY_REINTERPRET_CAST(T, expr) ((T) (expr))
#endif
#if defined(HEDLEY_STATIC_CAST)
# undef HEDLEY_STATIC_CAST
#endif
#if defined(__cplusplus)
# define HEDLEY_STATIC_CAST(T, expr) (static_cast<T>(expr))
#else
# define HEDLEY_STATIC_CAST(T, expr) ((T) (expr))
#endif

@rathann rathann requested a review from mr-c June 29, 2025 19:23
rathann and others added 3 commits June 30, 2025 18:23
Fixes:
```
error: implicit conversion changes signedness: 'uint32_t' (aka 'unsigned int') to 'int32_t' (aka 'int') [-Werror,-Wsign-conversion]
```

Co-authored-by: Michael R. Crusoe <1330696+mr-c@users.noreply.github.com>
@mr-c mr-c force-pushed the fix-signed-integer-overflow-ub branch from 6611e2c to d0b0b5f Compare June 30, 2025 16:23
@mr-c mr-c enabled auto-merge (squash) June 30, 2025 16:24
@mr-c mr-c merged commit 09bdaca into simd-everywhere:master Jun 30, 2025
112 checks passed
@rathann rathann deleted the fix-signed-integer-overflow-ub branch June 30, 2025 20:55
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.

Many intrinsics exhibit signed integer overflow UB

3 participants