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
rbind unmatched columns crashes R session (Windows, dev version) #4402
Comments
Thanks for reporting.
|
This line leads to the crash. Lines 210 to 211 in dd7609e
|
I can confirm it's due to the Chinese translation because when I set the language to |
Looks like we can't use Lines 210 to 212 in dd7609e
to if (usenames==TRUE) error( _("Column %d ['%s'] of item %d is missing in item %d. Use fill=TRUE to fill with NA (NULL for list columns), or use.names=FALSE to ignore column names.%s"),
w2+1, str, i+1, missi+1, extra); The crash will no longer happen. If that's the case, we may have to check all the combination of |
I'm almost sure that As you can see that the following code (another place uses library(data.table)
DT = data.table(a=1:3,b=2)
rbind(DT,list(b=5,a=3), use.names = F) MacWindows |
Thanks for diving on this @shrektan. Have asked about this on r-devel, it's a bit above my pay grade. https://stat.ethz.ch/pipermail/r-devel/2020-April/079397.html We would drop translation for such messages if there are no suggestions there... I also didn't see any thing googling around for In the meantime, I tried to replicate your error on Appveyor on the |
What if the size of chinesse characters is so big that it exceeds the size allocated for buff? |
I believe I am no expert here at all but I don't see how it could work otherwise b/c of the buffer overflow issue. (FWIW Chinese messages will usually be "narrower" than the English translation but maybe not in edge cases) |
@jangorecki Yes, as you said, the buff size is not the cause. I actually tried to increase the buff size and it doesn't fix this issue as expected.
@MichaelChirico If the virual machine can't be set to the Chinese locale, you may try to add a line |
I tried to reproduce this locally, but no luck so far. I built R-devel with ASAN which usually catches crashes like this. @shrektan when you say 'crash' is there any message at all or does R.exe just simply stop running and exit. Can you try R.exe at the prompt outside RStudio and see if that makes any difference or gets us any sort of error? Sounds like you're able to compile and try changes out ... if you change the
That comment seems significant. The order of the arguments in that string (number, string, number, number, string) is different in the translation, which seems to be what the n$ prefix is for. If that's not working then a mixup in arguments to specifiers (a number where a string is expected) is just the sort of thing that can cause these hard to reproduce crashes. Anyway, here's what I get, see CRAN_Release.cmd for how I configured R-devel.
I was hoping to see some sort of memory exception printed out by ASAN with a line number to the Or maybe it truly is a Windows-only problem. |
@shrektan are you using 32bit or 64bit R on Windows? |
Yes, it's definitely a Windows-platform specific issue, in my opinion. Just reproduced it on another Windows computer:
The session info:My doubtIn addition, I doubt that it's a problem relates to Encoding because R has to convert the translated message to the proper native encoding in order to display them correctly. (I actually have no idea how the function |
Don't if that is related but I see Rterm on your screenshot.
|
Looks like numbered argument specifiers ( https://stackoverflow.com/questions/19327441/gcc-dollar-sign-in-printf-format-string That would explain why it works on Linux and Mac, but we see the crash on Windows. This is consistent with @shrektan's output and what he wrote above already. On Windows, looks like we can use https://docs.microsoft.com/en-us/cpp/c-runtime-library/printf-p-positional-parameters?view=vs-2019 |
Nice finds! Feels like it should be documented in base R, I'll try & file a patch... |
All I really did in in the end in that merged PR (#4523) was https://www.gnu.org/software/libc/manual/html_node/Feature-Test-Macros.html R itself seem to set _XOPEN_SOURCE via _GNU_SOURCE.
R does seem to have code to interpret |
Another nice find on My own reading per #4523 (comment) was that the Sorry for the unforeseen headaches of introducing translations! |
@mattdowle Thanks for all these hard investigations. Unfortunately, the dev version of data.table doesn't fix this issue on my computer. The issue is the same: R hangs for 1 or 2 seconds before the window gets closed silently. I've tried it on x64 / i386 with Rgui.exe / R.exe. All behave the same... |
Hmm at this point, we might consider just removing translation for We can return to getting those translated later... |
@shrektan Please re-test master with #4524 now merged. Thanks to you getting the Chinese to work on AppVeyor, and that test is passing now, fingers crossed! Will leave this issue open until you confirm. To do before closing:
|
@mattdowle I confirm the issue gets fixed - no crash with Chinese error message as expected. Thanks for all these! Can't believe it turns out to be such a complicated issue... and it's really amazing that you find out the root cause of this issue and fix it in such a short time. Thanks again :D! |
I'm also getting this compile warning on current master:
|
I am not getting warnings like this... gcc 7.5.0 here. |
|
Test 450 will crash R session with dev version of data.table (but not with CRAN version) on Windows.
R version: 3.6.2
Platform: Windows 10
The text was updated successfully, but these errors were encountered: