Bug 5995 – string append negative integer causes segfault

Digital Mars D Compiler v2.052 $ dmd SpawnBug.d $ ./SpawnBug Segmentation fault (core dumped) $ cat SpawnBug.d void main(){ string ret; int i = -1; ret ~= i; }

This may or may not be expected. Appending any non-Unicode (> 0x10ffff) character will halt the program. In _d_arrayappendcd (https://github.com/D-Programming-Language/druntime/blob/master/src/rt/lifetime.d#L1762, BTW why this is in rt/lifetime.d?!): else if (c <= 0x10FFFF) { ... } else assert(0); // invalid utf character - should we throw an exception instead?

Calling onUnicodeError would be more appropriate.

Hm... I think in general it is a design flaw to allow int to implicitly cast to dchar. I think that is the source of the problem. Going from (d|w)char to the appropriate width integer should be fine, but going the other way seems prone to error. Note that in lifetime.d, the assert(0) should not lead to a segmentation fault.

Implicitly converting to the same-size _unsigned_ integral type might be fine, but converting to a signed type would be a narrowing conversion. I'd still argue that converting between any of the character types and any of the integral types should require a cast though simply because they're not only different types, they're different types of types. The character types are for characters and the integral types are for integers. Regardless, no implicit conversion should be permitted when it's a narrowing conversion. Narrowing conversions should require casts.

(In reply to comment #3) > Hm... I think in general it is a design flaw to allow int to implicitly cast to > dchar. > > I think that is the source of the problem. > > Going from (d|w)char to the appropriate width integer should be fine, but going > the other way seems prone to error. > > Note that in lifetime.d, the assert(0) should not lead to a segmentation fault. assert(0) emits asm{hlt;} when compiled in release mode. Encountering hlt _is_ a segmentation fault, so this is just fine.

(In reply to comment #4) > Implicitly converting to the same-size _unsigned_ integral type might be fine, > but converting to a signed type would be a narrowing conversion. I'd still > argue that converting between any of the character types and any of the > integral types should require a cast though simply because they're not only > different types, they're different types of types. The character types are for > characters and the integral types are for integers. Regardless, no implicit > conversion should be permitted when it's a narrowing conversion. Narrowing > conversions should require casts. How is that "narrowing"? No information is lost. @Topic: void main(){ uint i=-1; //fine dchar c=-1; //compile time error }

dchar is unsigned. int is signed. They don't cover the same range of values. Converting from one to the other in either direction is a narrowing conversion. I expect that the only reason that uint i = -1; compiles is to make it easy to create the unsigned value whose equivalent is -1 or some other reason related to C code. But personally, I don't think that it should compile without a cast, because you cannot represent -1 in a uint.

(In reply to comment #6) > void main(){ > uint i=-1; //fine > dchar c=-1; //compile time error > } Just tried this and it indeed produces an error: Error: cannot implicitly convert expression (-1) of type int to dchar So I wonder why this works? Seems inconsistent: int i = -1; dchar c = i; Also, the reporter's issue seems to be inconsistent with that error.

Still present in 2.065.

Still present in git HEAD (2.067b).

Should appending invalid codepoints append the Unicode replacement character instead?

(In reply to hsteoh from comment #11) > Should appending invalid codepoints append the Unicode replacement character > instead? I think implicit casting of int to dchar should be invalid altogether. See my 2011 comment.

Still present in 2.067

This is a problem in the compiler. https://github.com/dlang/dmd/blob/master/src/dcast.d#L66 https://github.com/dlang/dmd/blob/master/src/mtype.d#L4150 I will open a PR shortly to disable implicit cast of int -> dchar. Should we disable the implicit cast of all integral types to chars? For example, is it expected to make an implicit cast from uint to dchar? (The compiler itself seems to rely on implicit casts of unit -> dchar. Compiling the compiler with this cast disabled produces some errors.) This is also enabled: bool -> dchar. Not sure if it is desirable. Expression.implicitCastTo(z of type bool) => dchar

Lucia, I think nothing should implicitly cast to dchar. Not bool, int, or even char or wchar. But something this drastic needs approval from Walter and Andrei. Of course, we definitely need a deprecation step before completely banning it -- this will certainly break a lot of code.

This bug has a simple fix - throw a runtime exception (e.g. by onUnicodeError) instead of assert(0). We shouldn't change language rules on account of this. Thanks!

There are two problems, one is that the OP's code compiles, the other is that it segfaults. Arguably, fixing the first problem will fix the second. But just fixing the second leaves other problems still intact. Also, note that this succeeds, but likely does not do what the writer wants: string s = "123456"; s ~= 7; Guess what this does (yes, it compiles)? s ~= 123456;

As Andrei suggested, here is the quick fix: PR https://github.com/dlang/druntime/pull/1696 Language design changes should be discussed with Walter and Andrei in depth.

Commits pushed to master at https://github.com/dlang/druntime https://github.com/dlang/druntime/commit/316e6d2607b4b22794ef75a331ad27d970717cda fix issue 5995 https://github.com/dlang/druntime/commit/6dbbadbac4a0567ba49f0e616fccc8c597fec771 Merge pull request #1696 from somzzz/issue_5995 fix issue 5995 - string append negative integer causes segfault

Commits pushed to scope at https://github.com/dlang/druntime https://github.com/dlang/druntime/commit/316e6d2607b4b22794ef75a331ad27d970717cda fix issue 5995 https://github.com/dlang/druntime/commit/6dbbadbac4a0567ba49f0e616fccc8c597fec771 Merge pull request #1696 from somzzz/issue_5995

Commits pushed to stable at https://github.com/dlang/druntime https://github.com/dlang/druntime/commit/316e6d2607b4b22794ef75a331ad27d970717cda fix issue 5995 https://github.com/dlang/druntime/commit/6dbbadbac4a0567ba49f0e616fccc8c597fec771 Merge pull request #1696 from somzzz/issue_5995

Commits pushed to newCTFE at https://github.com/dlang/druntime https://github.com/dlang/druntime/commit/316e6d2607b4b22794ef75a331ad27d970717cda fix issue 5995 https://github.com/dlang/druntime/commit/6dbbadbac4a0567ba49f0e616fccc8c597fec771 Merge pull request #1696 from somzzz/issue_5995

*** Issue 16545 has been marked as a duplicate of this issue. ***