Skip to content
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

setattr crashed when given empty character vector as name or value #2386

Closed
hatal175 opened this issue Sep 25, 2017 · 5 comments
Closed

setattr crashed when given empty character vector as name or value #2386

hatal175 opened this issue Sep 25, 2017 · 5 comments
Assignees
Labels
bug
Milestone

Comments

@hatal175
Copy link

@hatal175 hatal175 commented Sep 25, 2017

Trying to run setattr(numeric(1), "class", character(0)) or setattr(numeric(1), character(0), "value") crashes because the setattrib c function doesn't check if the value or name string vector actually has length > 0 before accessing it with STRING_ELT.

@franknarf1
Copy link
Contributor

@franknarf1 franknarf1 commented Sep 25, 2017

No repro on Win 7:

library(data.table)
setattr(numeric(1), "class", character(0))[]
# [1] 0
setattr(numeric(1), character(0), "value")[]
# Error in setattr(numeric(1), character(0), "value") : 
#   'installTrChar' must be called on a CHARSXP

with

data.table 1.10.5 IN DEVELOPMENT built 2017-09-23 01:51:19 UTC; travis
R version 3.3.3 (2017-03-06)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 7 x64 (build 7601) Service Pack 1
@hatal175
Copy link
Author

@hatal175 hatal175 commented Sep 25, 2017

This managed to crash mine (same for character(0) in the other parameter):
xx = lapply(1:10000, function(x) setattr(numeric(1), "class", character(0)))

R version 3.4.1 (2017-06-30)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 15063)

data.table 1.10.4

@franknarf1
Copy link
Contributor

@franknarf1 franknarf1 commented Sep 25, 2017

I guess you should test on the development version before reporting here, per https://github.com/Rdatatable/data.table/wiki/Support

@hatal175
Copy link
Author

@hatal175 hatal175 commented Sep 25, 2017

Trying this with the R-devel version and latest data.table build does not reproduce. But here's the thing, I'm pretty sure this is dependent on what memory the uninitialized data is pointing to. Different builds might do different things. However, the bug I pointed out is still there and it is a simple code flow.

@eantonya eantonya added the bug label Sep 28, 2017
@MarkusBonsch MarkusBonsch self-assigned this Feb 14, 2018
@MarkusBonsch
Copy link
Contributor

@MarkusBonsch MarkusBonsch commented Feb 14, 2018

I can crash with
xx = lapply(1:10000, function(x) setattr(numeric(1), "class", character(0)))
but not with
xx = lapply(1:10000, function(x) setattr(numeric(1), "bla", character(0)))
and
setattr(numeric(1), character(0), "value")
throws a normal error as expected.

So, the only problem is when using setattr on the class attribute with 0 length value.
This may well have been fixed in the current R-devel, since data.table is just calling the R setAttrib function here. However, I will still fix this issue on data.table side just to be sure.

Cheers,
Markus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.