Skip to content

growVector, copyAsPlain may formally cause undefined behaviour with 0-length vectors #6819

Closed
#6820
Closed
growVector, copyAsPlain may formally cause undefined behaviour with 0-length vectors#6819
#6820

Description

@aitap
aitap
opened on Feb 17, 2025

Found while working on a custom Debian testing-based container (using C compiler: ‘Debian clang version 19.1.7 (1+b1)’) with Clang sanitizers enabled for #6746:

When trying to access the contents of a zero-length vector using INTEGER(...) or REAL(...) or other accessor, R may return an invalid pointer (0x1). The C standard says that giving an invalid pointer to memcpy() is undefined behaviour, even though in practice nothing breaks (memcpy sees n=0 and doesn't dereference it).

If nothing breaks, what's the risk? One of the CRAN special checks (clang-UBSAN or 0len) might pick this up too. Very far-fetched, a compiler might optimize away a chunk of code deemed to cause undefined behaviour.

Running test id 173.1

dogroups.c:541:39: runtime error: load of misaligned address 0x000000000001 for type 'int *', which requires 4 byte alignment
0x000000000001: note: pointer points here
<memory cannot be printed>
    #0 0x7f2efbaec116 in growVector /work/data.table.Rcheck/00_pkg_src/data.table/src/dogroups.c
    #1 0x7f2efbae90d5 in dogroups /work/data.table.Rcheck/00_pkg_src/data.table/src/dogroups.c:409:66

(gdb) frame 4
#4  0x00007fc6d4aec117 in growVector (x=0x52500661b038, newlen=newlen@entry=3) at dogroups.c:543
543       case CPLXSXP: memcpy(COMPLEX(newx), COMPLEX(x), len*SIZEOF(x)); break;
(gdb) p Rf_xlength(x)
$4 = 0
(gdb) p Rf_xlength(newx)
$5 = 3
(gdb) call Rf_PrintValue(R_GlobalContext->call)
`[.data.table`(DT, , B[B > 3], by = A)

Running test id 893.5

utils.c:233:12: runtime error: store to misaligned address 0x000000000001 for type 'int *', which requires 4 byte alignment
0x000000000001: note: pointer points here
<memory cannot be printed>
    #0 0x7f2efbbfd5f9 in copyAsPlain /work/data.table.Rcheck/00_pkg_src/data.table/src/utils.c:233:5
    #1 0x7f2efbbefbf9 in subsetDT /work/data.table.Rcheck/00_pkg_src/data.table/src/subset.c:317:30

(gdb) frame 4
#4  0x00007fc6d4bfd5fa in copyAsPlain (x=x@entry=0x525003c44b68) at utils.c:233
233         memcpy(INTEGER(ans), INTEGER(x), n*sizeof(int));             // covered by 10:1 after test 178
(gdb) p Rf_xlength(x)
$8 = 0
(gdb) call Rf_PrintValue(R_GlobalContext->call)
`[.data.table`(head(DT, nr), , seq_len(if (nc == 0) ncol(DT) else nc),
    with = FALSE)

Running test id 2150.21

dogroups.c:540:39: runtime error: load of misaligned address 0x000000000001 for type 'int *', which requires 4 byte alignment
0x000000000001: note: pointer points here
<memory cannot be printed>
    #0 0x7f2efbaec116 in growVector /work/data.table.Rcheck/00_pkg_src/data.table/src/dogroups.c
    #1 0x7f2efbb46d4e in allocateDT /work/data.table.Rcheck/00_pkg_src/data.table/src/freadR.c:501:36
    #2 0x7f2efbb2f967 in freadMain /work/data.table.Rcheck/00_pkg_src/data.table/src/fread.c:2666:7
    #3 0x7f2efbb42306 in freadR /work/data.table.Rcheck/00_pkg_src/data.table/src/freadR.c:222:3

(gdb) frame 4
#4  0x00007fc6d4aec117 in growVector (x=x@entry=0x525004e417e8, newlen=newlen@entry=1024)
    at dogroups.c:543
543       case CPLXSXP: memcpy(COMPLEX(newx), COMPLEX(x), len*SIZEOF(x)); break;
(gdb) p Rf_xlength(x)
$11 = 0
(gdb) call Rf_PrintValue(R_GlobalContext->call)
fread("c1\n2018-01-31 03:16:57")

(Yes, that case CPLSXP: looks a bit strange. clang must have merged the branches into one with different length arguments to memcpy().)

Activity

  1. aitap
    closed this as completedin #6820on Feb 23, 2025

  2. MichaelChirico commented on Apr 16, 2025

    @MichaelChirico
    MichaelChirico
    on Apr 16, 2025
    Member

    Funnily enough I managed to come across this same bug in a "different" setting, but I still can't reproduce it (even under --enable-strict-barrier) locally.

    Could you share more details of the container you were using?

    Does {vctrs} also segfault there? (I think it does: r-lib/vctrs#1968)

  3. aitap commented on Apr 16, 2025

    @aitap
    aitap
    on Apr 16, 2025
    MemberAuthor

    vctrs must be encountering some other problem. Could it be due to ABI changes? Some package needing to be reinstalled?

    Here we had r-devel-clang-san complaining that calling memcpy(0x1, 0x1, 0) is formally undefined behaviour even though no modern memcpy() dereferences the pointers when n == 0. (Moreover, starting with C23, memcpy() with n == 0 will be required to be a no-op, no matter whether the pointers are valid or notmemcpy(0,0,0) will be a no-op, which is not relevant here.) It wouldn't crash without the sanitizer.

    If this was a zero-length access, the crash would be due to access to 0x1 or some other low-value pointer, not 0x7f405afb3a6a (which looks like mmap area, so probably a large heap allocation).

    Still, the fix shouldn't hurt and may prevent an UBSan warning during CRAN checks.

  4. MichaelChirico commented on Apr 16, 2025

    @MichaelChirico
    MichaelChirico
    on Apr 16, 2025
    Member

    The only thing we did is update R 4.4.1 --> R 4.5.0. It seems that is turning on stricter checks in R:

    https://github.com/r-devel/r-svn/blob/d62355fdacfd9a9a41639134ee9875e0c2502ca7/src/include/Rinlinedfuns.h#L84
    https://github.com/r-devel/r-svn/blob/d62355fdacfd9a9a41639134ee9875e0c2502ca7/src/main/memory.c#L31
    https://github.com/r-devel/r-svn/blob/d62355fdacfd9a9a41639134ee9875e0c2502ca7/src/main/memory.c#L4146

    Possibly the non-0x1 address is from (void*)1, though I admittedly still don't fully understand the issue, possibly we just have stricter implementations on by default.

  5. aitap commented on Apr 16, 2025

    @aitap
    aitap
    on Apr 16, 2025
    MemberAuthor

    I couldn't get it to crash. I also tried installing all the vctrs dependencies on R 4.4.1, then running R 4.5.0 --with-strict-barrier without rebuilding them. (The latter only got me two test failures of the form HASHTAB: argument of type SYMSXP is not an environment or NULL.) Any other information that could help reproduce the crash? My compiler is probably too old. A gdb backtrace could also be informative.

    While the C standard allows making a pointer out of a number to perform very wild transformations, on our today's computers with MMUs and flat address spaces, (void*)1 stores a 1 in the register (or memory).

  6. aitap commented on Apr 18, 2025

    @aitap
    aitap
    on Apr 18, 2025
    MemberAuthor

    Are you compiling with -fsanitize-trap? This is what -fsanitize-trap looks like: r-hub/containers#84

    CRAN special tests may be disabling the memcpy test, or running their tests in C23 mode, where memcpy(..., ..., 0) should no longer count as load/store (but I haven't verified either option)or using an older version of the compiler which doesn't have this check enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions