dmd icon indicating copy to clipboard operation
dmd copied to clipboard

Fix issue 22828: dcast: use an implicit cast instead of explicit

Open ljmf00 opened this issue 3 years ago • 4 comments

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.

ljmf00 avatar Feb 27 '22 21:02 ljmf00

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 ⚠️⚠️⚠️

To target stable perform these two steps:

  1. Rebase your branch to upstream/stable:
git rebase --onto upstream/stable upstream/master
  1. Change the base branch of your PR to stable

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"

dlang-bot avatar Feb 27 '22 21:02 dlang-bot

@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.

RazvanN7 avatar Feb 28 '22 10:02 RazvanN7

@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 avatar Feb 28 '22 15:02 ljmf00

@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.

ljmf00 avatar Feb 28 '22 15:02 ljmf00