`#embed` doesn't support reading from devices

[zygoloid]

Clang's #embed implementation doesn't behave properly when applied to a device:

int per_build_entropy[] = {
#embed "/dev/random" limit(1024)
};

Instead it treats devices as being empty -- in the above case, this leads to the array being initialized with zeroes! This is probably because we're using the st_size field produced by stat, but that only lists the size of the file if applied to a regular file:

st_size

This field gives the size of the file (if it is a regular file or a symbolic link) in bytes. The size of a symbolic link is the length of the pathname it contains, without a terminating null byte.

While we try to detect this case and use stream IO instead of mmap, we only do so when we weren't given a MapSize, which in this case we presumably will be due to the limit(1024). And the code that handles the case where we have no MapSize does not handle the case where we're opening a device rather than a regular file. Perhaps shouldUseMmap should return false unless it's operating on a regular file?

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Richard Smith (zygoloid)

Clang's `#embed` implementation doesn't behave properly when applied to a device: ```c++ int per_build_entropy[] = { #embed "/dev/random" limit(1024) }; ``` Instead it treats devices as being empty -- in the above case, this leads to the array being initialized with zeroes! This is probably because we're using the `st_size` field produced by `stat`, but that only lists the size of the file if applied to a regular file: > `st_size` > > This field gives the size of the file (if it is a regular file or a symbolic link) in bytes. The size of a symbolic link is the length of the pathname it contains, without a terminating null byte.

While we try to detect this case and use stream IO instead of mmap, we only do so when we weren't given a MapSize, which in this case we presumably will be due to the limit(1024). And the code that handles the case where we have no MapSize does not handle the case where we're opening a device rather than a regular file. Perhaps shouldUseMmap should return false unless it's operating on a regular file?

[shafik]

CC @AaronBallman @Fznamznon

[Fznamznon]

While we try to detect this case and use stream IO instead of mmap, we only do so when we weren't given a MapSize, which in this case we presumably will be due to the limit(1024)

Not exactly, limit is not passed for #embed to file reading APIs. We simply cut the data that is already read when limit parameter is present. What happening is, FileSize is asked in FileManager::getBufferForFile and ends up being 0 for a character device and then passed as MapSize, so MapSize is 0 and we don't read anything. I'm looking.

[zygoloid]

I guess we'll probably want to start passing a size if we want things like this to work, given that MemoryBuffer doesn't support a buffer of unknown size / a buffer filled on demand. FWIW, while this example is contrived, it does work in GCC, so I'd expect we'll eventually see some use of techniques like this in the wild.

[Fznamznon]

Whereas an example with embedding a character device file is present in the original #embed paper (and somehow changed yet similar example with an "endless file" is present in the most recent C working draft that I could find), there are still several questions to how this should work. For example what to do if limit parameter is not present? What to do with clang::offset parameter when reading from a device?
I also stumbled upon this comment #68620 (comment) from @cor3ntin .
I was able to make #embed "/dev/random" limit(1024) work but embedding "/dev/null" looks incorrect with limit parameter and I don't think this it is doable without modifying FileManager/VFS/getOpenFileImpl interfaces or introducing new APIs because getOpenFileImpl unconditionally creates a Buffer of some size and fills it with zeros even though there is no content to read here

llvm-project/llvm/lib/Support/MemoryBuffer.cpp

Line 539 in 3c6d1dd

std::memset(ToRead.data(), 0, ToRead.size());

. Before doing that I would double check with folks that we need this.
CC also @AaronBallman @Endilll @cor3ntin @jyknight for opinions .

[Endilll]

I think it's worth pointing out to a paper about offset parameter which was forwarded to EWG (https://wg21.link/p3540), but there seems to be controversy on what exactly EWG has approved (http://lists.isocpp.org/ext/2025/02/25228.php).

[zygoloid]

For example what to do if limit parameter is not present?

This part at least is not an entirely new problem -- the same concern applies to #include, as well as various other ways that Clang can be given filenames of input files. FWIW, it looks like GCC reads from the file until it runs out of memory and crashes for both #include "/dev/zero" and #embed "/dev/zero". That's not great behavior, but perhaps we can try to detect when we've read an unreasonable amount and bail out with a "file too large" error?

What to do with clang::offset parameter when reading from a device?

We use the ::read function to read the data when reading from a device, so it seems appropriate to use ::lseek to skip past the offset.

I also stumbled upon this comment #68620 (comment) from @cor3ntin.

As demonstrated in the first comment on this bug, our current implementation is a potential security hazard. If we want to reject device files entirely, I think that might mostly be a reasonable position, except that GCC allows them, the proposal clearly intended to support them, and so it seems likely that rejecting them will eventually cause compatibility problems for someone.

I was able to make #embed "/dev/random" limit(1024) work but embedding "/dev/null" looks incorrect with limit parameter and I don't think this it is doable without modifying FileManager/VFS/getOpenFileImpl interfaces or introducing new APIs

Yeah, it looks like we would need to plumb support for this through several layers of LLVM. As far as I can see, that's part of the necessary work to fully implement this feature, though, so I guess that's what we need to do, unless we think we can get away with rejecting.

[AaronBallman]

perhaps we can try to detect when we've read an unreasonable amount and bail out with a "file too large" error?

We might end up needing a driver flag to let the user decide what constitutes "too large" given that embed is expected to handle large amounts of data. At least initially, "read until the compiler crashes" won't accidentally limit users who don't use limit (pun intended).

the proposal clearly intended to support them, and so it seems likely that rejecting them will eventually cause compatibility problems for someone.

I'd like to avoid rejecting them as they're a valid use case that should work.

Is there an API to determine whether a device file is one which is "endless?" If so, perhaps we could error if there's no limit parameter when using such a file.

[Fznamznon]

Is there an API to determine whether a device file is one which is "endless?" If so, perhaps we could error if there's no limit parameter when using such a file.

We can understand that we're dealing with a character device file or a named pipe. I haven't seen an implemented way to see if it is "endless".