Fix issue 10398 #2486
Fix issue 10398 #2486
Conversation
| @@ -296,8 +296,8 @@ void Lexer::deprecation(const char *format, ...) | |||
| } | |||
|
|
|||
| TOK Lexer::nextToken() | |||
| { Token *t; | |||
|
|
|||
| { Token *t = (Token*)mem.malloc(sizeof(Token));; | |||
There was a problem hiding this comment.
This fix is likely wrong (t is only used in the token.next branch), and would be performance suicide either way.
There was a problem hiding this comment.
It is later saved in freelist global, but it is possible to move allocation inside if(token.next).
There was a problem hiding this comment.
It seems as if the return value of mem.malloc is never used in your code, as t is immediately set to token.next in the respective branch.
I haven't touched that part of DMD in a while, but what is supposed to happen here is that the next token is copied into the space for the currently processed one, and then the allocation for the former is discarded. Discarded here means adding it to the manually maintained free list (as we have to avoid allocation overhead here at any cost for performance).
In case token.next can point to garbage, this obviously is a problem that needs to be fixed.
There was a problem hiding this comment.
I agree, this looks wrong and is not solving anything. If t is required to be initialised, use NULL. Or even better, move the declaration into the true branch. Token *t = token.next;.
|
@klickverbot @ibuclaw Yes. I removed allocation. |
| if (token.next) | ||
| { | ||
| t = token.next; | ||
| Token *t = token.next; |
There was a problem hiding this comment.
I don't see how this addresses:
2.in Lexer::nextToken() *t pointer is uninitialized and its value is later copied.
|
Ok, closed. |
ghost
closed this
http://d.puremagic.com/issues/show_bug.cgi?id=10398
These changes fix some memory issues in dmd detected by memcheck during druntime and phobos compilation.