1684087 - warning: specified bound 3 equals destination size in nsMsgTemplateReplyHelper::OnDataAvailable

Status: NEW

(MailNews Core :: Composition, defect)

Description

[:aceman]

When compiling Thunderbird with GCC 10.2.0 I get several warnings of this class:
In function ‘char* strncpy(char*, const char*, size_t)’,
inlined from ‘virtual nsresult nsMsgTemplateReplyHelper::OnDataAvailable(nsIRequest*, nsIInputStream*, uint64_t, uint32_t)’ at comm/mailnews/compose/src/nsMsgComposeService.cpp:813:16:
/usr/include/bits/string_fortified.h:106:34: warning: ‘char* __builtin_strncpy(char*, const char*, long unsigned int)’ specified bound 3 equals destination size [-Wstringop-truncation]
106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
| ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I am wondering what the problem is here.

This is the single occurrence in Thunderbird, but there are about 5 others in m-c code.

Comment 1

[Magnus Melin [:mkmelin]]

I think it's tying to say there's no room for trailing null considered.
https://patchwork.ozlabs.org/project/gcc/patch/[e-mail]/

Comment 2

[:aceman]

Kinda yes, but isn't that expected in strncpy() ? It copies the specified number of bytes whether they included the trailing null or not.

The code line is like this:
mInMsgBody = bodyOffset != 0;
if (!mInMsgBody && readCount > 3) // still in msg hdrs
strncpy(mLastBlockChars, readBuf + readCount - 3, 3);

Comment 3

[Magnus Melin [:mkmelin]]

http://www.cplusplus.com/reference/cstring/strncpy/ says "destination and source shall not overlap" which it looks like they might do in this case? https://searchfox.org/comm-central/source/mailnews/local/src/nsLocalMailFolder.cpp#1942
Perhaps memmove should be used instead.

Comment 4

[:aceman]

How do they overlap? mLastBlockChars and readBuf do not seem related.

On the other hand, just a few lines above we copy mLastBlockChars back into readBuf as 'memcpy(readBuf, mLastBlockChars, 3);'.
strncpy() does zero the trailing chars if the src string is shorten than the length, but it seems at the code here we know the src is at least 3 bytes, so memcpy and strncpy could do the same.

Comment 5

[:aceman]

My other theory of why the compiler warns would be that if we copy a string of 3 chars into a 3-char mLastBlockChars and it contains no null, then we can't read that string back using string functions as we would run off the end of the 3-char array. Yes, we probably don't do that, as we do not run string reading on that array later on (as we know it is not a safe string), so then why do we copy it using a string function in the first place (strncpy)? Maybe that's what the compiler means and that when it isn't really a string, we should use memcpy or similar.

Comment 6

[ISHIKAWA, Chiaki]

I would vote for memmove() [or bcopy() but that would end up as the same built-in function, I think.].