Created attachment 1657975 [details]
preprocessed source

For sure, preprocessed source

Well, actually "strncat" (not strncpy)

What do you find wrong on that?  The warning (added in http://gcc.gnu.org/PR81117) warns that what you are doing is very unlikely what you want,
because it will keep the resulting string unterminated.
strncat (x, y, strlen (y)) is effectively memcpy (strchr (x, '\0'), y, strlen (y)).

No. "man 3p strncat" actuall says:

The strncat() function shall append not more than n bytes (a NUL character and bytes that follow it are not appended)
from the array pointed to by s2 to the end of the string pointed to by
s1.  The initial byte of s2 overwrites the NUL character at the end of s1.  

A terminating NUL character is always appended to the result. 
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
If copying takes place between objects that  overlap, the behavior is undefined.

Actually sorry, ignore previous comment, it is zero terminated, the warning is just about it being completely pointless to call strncat in that case, because strncat (x, y, strlen (y)) will do exactly what strcat (x, y) does, so why bother
with the extra obfuscation?

Well, actually the above is the minimum case and the original code is like

----------------------------------------------------------------
void append( char *dst, const char *src) {
  append_with_length(dst, src, strlen(src));
}

void append_with_length(char *dst, const char *src, size_t len) { strncat(dst, src, len); }
----------------------------------------------------------------

(actually append_with_length() does some extra works) and gcc inlines these and causes the warning.
So originally there is seperated functions like above.

Created attachment 1657978 [details]
very original source

For reference, very original source

Martin Sebor, the author of this warning, is on CC, so I'll let him explain the intentions of the warning.

The expected use of strncat is to constrain the copy to the space remaining in the destination to avoid buffer overflow.  Passing in as the bound the length or size of the source of the copy defeats the function's intended purpose and is a common source of bugs.  The following US CERT article has some helpful background, including guidance for how to safely use the function: https://www.us-cert.gov/bsi/articles/knowledge/coding-practices/strncpy-and-strncat

ISTM this warning is working precisely how it should be.  IMHO this should be CLOSED/NOTABUG.

The code, as written, certainly looks buggy.  The length argument to a strncat should specify the *remaining bytes in the destination*.  Having it be the length of the source string is just wrong unless there is some restriction on the length of the source string which ensures it's smaller than the remaining bytes in the destination.

ISTM this code could use strcat rather than strncat with absolutely no change in its security posture.

> The length argument to a strncat should specify the *remaining bytes in the destination*.  

Actually for the original source, dst is actually realloced and remaining bytes in the destination is specified. 
==================================================================
void
append (char **dest, const char *src)
{
	size_t  len;
	len = strlen (src);
	appendn (dest, src, len);
}

void
appendn (char **dest, const char *src, size_t len)
{
	size_t  total;
	/* +1 for terminating nul */
	total = strlen (*dest) + 1;
	total += len;
	*dest = realloc (*dest, total);
	strncat (*dest, src, len);
}
==================================================================

So the above strncat() usage in appendn is correct.

Both strncpy and strncat are broken by design, each differently.
But, with the above, can you explain why do you use strncat at all?
It is also inefficient, especially with large strlen (*dest).
Why don't you instead
size_t dest_len = strlen (*dest);
size_t total = dest_len + len + 1;
*dest = realloc (*dest, total);
memcpy (*dest + dest_len, src, len);
(*dest)[dest_len + len] = '\0';
?  If you have determined strlen (*dest) already, having to walk it again during strncat is a waste of CPU cycles.

(
First of all, to make it sure, what I am doing here is that on devel mailing list someone complained about this warning: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/5EKIEJLEVIEHZ33B5BTPEPZPJK2O5IIV/
and I just analyzed this and reported here. I already attached the original source, which is in "procenv" srpm.
)

(
Well, let's not argue about "waste of CPU cycles" or "efficiency" or so, that will bring the discussion to the wrong direction.
I don't want to discuss human preference here.
If compiler thinks "it is waste of CPU cycles", so compiler should warn *so*, not "-Wstringop-truncation".
So again I don't want to do such discussion here. Personally I am surprised that you suggest using memcpy for
handling string instead of using C library function: using memcpy seems more "raw" (direct) manipulation and more
dangerous, however I don't want to discuss such things here
)

(In reply to Mamoru TASAKA from comment #13)
> So again I don't want to do such discussion here. Personally I am surprised
> that you suggest using memcpy for
> handling string instead of using C library function: using memcpy seems more
> "raw" (direct) manipulation and more
> dangerous, however I don't want to discuss such things here
> )

But the whole point of the warning is to flag usages that are potentially mistakes. Using strncat here is **not** any safer than using memcpy or strcat. Getting a warm fuzzy feeling for using the "right" API doesn't make the code better. If the buffer is already guaranteed to be large enough (because it was just reallocated) then strcat (or memcpy) does the right thing.

(In reply to Martin Sebor from comment #9)
> The expected use of strncat is to constrain the copy to the space remaining
> in the destination to avoid buffer overflow.  Passing in as the bound the
> length or size of the source of the copy defeats the function's intended
> purpose and is a common source of bugs.  The following US CERT article has
> some helpful background, including guidance for how to safely use the
> function:
> https://www.us-cert.gov/bsi/articles/knowledge/coding-practices/strncpy-and-
> strncat

The text of the warning doesn't make it very clear what the problem is though.

It's just as well I checked this.  I'd have reported it if it didn't likely have the same cause as another case that appeared to be getting array bounds wrong.  (I've been looking at other packages which are now broken, and hadn't got round to posting initially on devel@fedora.)

The point is that it shows up in default Fedora builds (-O2) only on s390x, but on all the other targets at -O3, like the other case.  That's presumably not intended, regardless of how sensible the code is.

Without doing some digging, I think this is the warning introduced in GCC 9 which procenv got fixed to avoid. (It's built with -Werror.)  law suggested a fix from initial pre-release builds, but I can't remember whether that got used or if it was re-written differently.

(In reply to Jonathan Wakely from comment #15)
> 
> The text of the warning doesn't make it very clear what the problem is
> though.

You're right.  The warning in this case should be:
  specified bound depends on the length of the source argument [-Wstringop-overflow=]
Let me look into that.

This bug appears to have been reported against 'rawhide' during the Fedora 32 development cycle.
Changing version to 32.

I have adjusted GCC to issue the following warning instead.  With that, I think this bug can be resolved.

$ gcc -D_FORTIFY_SOURCE=2 -O2 -S -Wall -Wextra rhbz1798636.c
In file included from /usr/include/string.h:494,
                 from rhbz1798636.c:1:
In function ‘strncat’,
    inlined from ‘append’ at rhbz1798636.c:6:2:
/usr/include/bits/string_fortified.h:136:10: warning: ‘__builtin_strncat’ specified bound depends on the length of the source argument [-Wstringop-overflow=]
  136 |   return __builtin___strncat_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
rhbz1798636.c: In function ‘append’:
rhbz1798636.c:6:2: note: length computed here
    6 |  strncat(dst, src, strlen(src));
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I don't think it should be considered resolved if it's still inconsistent between -O2/-O3 and different targets.

Middle-end warnings to depend on optimization levels, targets, amount of inlining and many other things, there is no way around that.

This message is a reminder that Fedora 32 is nearing its end of life.
Fedora will stop maintaining and issuing updates for Fedora 32 on 2021-05-25.
It is Fedora's policy to close all bug reports from releases that are no longer
maintained. At that time this bug will be closed as EOL if it remains open with a
Fedora 'version' of '32'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 32 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Fedora 32 changed to end-of-life (EOL) status on 2021-05-25. Fedora 32 is
no longer maintained, which means that it will not receive any further
security or bug fix updates. As a result we are closing this bug.

If you can reproduce this bug against a currently maintained version of
Fedora please feel free to reopen this bug against that version. If you
are unable to reopen this bug, please file a new report against the
current release. If you experience problems, please add a comment to this
bug.

Thank you for reporting this bug and we are sorry it could not be fixed.