Skip to content
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

Revert reinterpret cast introduced in #432, add ICE test case for 32-bit targets #11546

Merged
merged 2 commits into from
Aug 11, 2020

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Aug 10, 2020

An ICE was seen when running the testsuite for a 16-bit target. This ICE was indirectly fixed in #9282 (d6139e3), however you could the same ICE on 32-bit if int[] was swapped with long[].

The reason why an ICE would happen is because the condition:

if (val.type.ty == Tsarray && pointee.ty == Tarray && elemsize == pointee.nextOf().size())

Would also inadvertently match int[] and int[][2] on 16-bit, long[] and long[][2] on 32-bit (and if there was support, cent[] and cent[][2] on 64-bit), and generate the wrong code for interpreting the assignment expression.

However looking further into it, the code that introduced the ICE in #432 made it possible for reinterpret casts that would have invalid results at run-time to yield a sensible result at CTFE. So that change has been reverted too. The original reason for why it was necessary is no longer relevant any more.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @ibuclaw!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#11546"

Internally these would treat the cast same as a normal conversion from
int[7] to int[], which allows code at CTFE to erroneously succeed where
it would raise a SEGV at run-time.
@ibuclaw ibuclaw changed the title compilable: Add ICE test case for 32-bit targets Revert reinterpret cast introduced in #432, add ICE test case for 32-bit targets Aug 10, 2020
@dlang-bot dlang-bot merged commit e9cc3dd into dlang:master Aug 11, 2020
@ibuclaw ibuclaw deleted the icem32 branch August 11, 2020 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants