dmd
dmd copied to clipboard
Fix issue 22828: dcast: use an implicit cast instead of explicit
Signed-off-by: Luís Ferreira [email protected]
I made this critical, but I don't know the definition of critical/major, so please clarify and change accordingly, if necessary. For context, this behaviour is being used on the backend code, and can trigger a silent buffer overflow, since the compiler explicit cast this wrongly. One example is on the object file generation, you can see that this triggers an error when using IndexExp but not with AddExp.
Thanks for your pull request, @ljmf00!
Bugzilla references
| Auto-close | Bugzilla | Severity | Description |
|---|---|---|---|
| ✓ | 22828 | critical | Compiler allow offset a pointer with types of size greater than sizeof(T*) |
⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️
- Regression or critical bug fixes should always target the
stablebranch. Learn more about rebasing tostableor the D release process.
To target stable perform these two steps:
- Rebase your branch to
upstream/stable:
git rebase --onto upstream/stable upstream/master
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#13732"
@ljmf00 What is the basis of your assumption that this should be illegal? You are doing pointer arithmetic in @system code, it is not the job of the compiler to prevent you from doing something like this. I don't think the bug report is valid.
@ljmf00 What is the basis of your assumption that this should be illegal? You are doing pointer arithmetic in @System code, it is not the job of the compiler to prevent you from doing something like this. I don't think the bug report is valid.
This goes against the specification, on many levels. The main problem is the fact that the compiler is doing explicit casting, implicitly, which is wrong and dangerous. Doing an explicit cast on types that can't implicitly promote can trigger a silent overflow on the offset buffer pointer. The compiler should warn about this and the user should do an explicit cast if he wants to proceed, similar to what is being done on IndexExp.
10.15.1.2. Pointer Arithmetic: IndexExpression can also be used with a pointer and has the same behaviour as adding an integer.
This is currently not being followed, since the current behaviour of IndexExp is what this PR tries to implement.
10.15.1. Add Expressions: If the operands are of integral types, they undergo the Usual Arithmetic Conversions, and then are brought to a common type using the Usual Arithmetic Conversions.
If the pointer is treated as an integer, which is on IndexExp, the Usual Arithmetic Conversions should be followed.
We can definitely clarify the specification about this, but doing (cast(uint)(uint + ulong)) is not good to do implicitly, especially, when dealing with memory addresses. Dereferencing a silent overflowed memory address can lead to catastrophic situations.
@ljmf00 What is the basis of your assumption that this should be illegal? You are doing pointer arithmetic in @System code, it is not the job of the compiler to prevent you from doing something like this. I don't think the bug report is valid.
Also, this has nothing to do with @safe/@system semantics.