Bug 88059 - Spurious stringop-overflow warning with strlen, malloc and strncpy
Summary: Spurious stringop-overflow warning with strlen, malloc and strncpy
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 8.2.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks: strlen Wstringop-overflow
  Show dependency treegraph
 
Reported: 2018-11-16 14:16 UTC by Paul-Antoine Arras
Modified: 2022-10-23 00:24 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-07-02 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paul-Antoine Arras 2018-11-16 14:16:32 UTC
The following combination of `strlen`, `malloc` and `strncpy` results in a spurious warning when compiling with optimisation (at least -O2):

```
$ cat bug.c

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char* copy_name (const char* src)
{
	size_t len = strlen(src) + 1;
	char* dest = malloc(len);
	if (dest)
		strncpy(dest, src, len);
	return dest;
}

int main (void)
{
	const char* name = "Name";
	char* copy = copy_name(name);
	printf("%s\n", copy);
	return 0;
}

$ gcc -O2 bug.c

bug.c: In function ‘copy_name’:
bug.c:10:3: warning: ‘strncpy’ specified bound depends on the length of the source argument [-Wstringop-overflow=]
   strncpy(dest, src, len);
   ^~~~~~~~~~~~~~~~~~~~~~~
bug.c:7:15: note: length computed here
  size_t len = strlen(src) + 1;
               ^~~~~~~~~~~

```
This is actually safe since the result of `strlen` is used for both `malloc` and `strncpy`.
Comment 1 Martin Sebor 2018-11-16 18:03:39 UTC
The warning is by design.  Quoting a comment from the code, it "triggers for LEN arguments that in any meaningful way depend on strlen(SRC)."

The warning in these cases is designed to detect the following anti-pattern:

  strncpy (dest, src, strlen (src));

The code in the test case in comment #0 happens to be safe but calling strncpy in this instance is not the intended use of the API: when the size of the destination is known to be sufficient for the copy, the strcpy function is more appropriate.

It might be possible to avoid the warning for a subset of these safe cases by trying to also determine whether DEST depends on LEN in the same way as LEN depends on SRC but in light of the above it doesn't seem worth the effort.
Comment 2 Moritz Bender 2019-07-02 18:52:15 UTC
I don't know whether this comment will ever be read, but I'd like to add that I have the same problem with strncpy incorrectly getting the "warning: 'strncpy' specified bound depends on the length of the source argument". I do believe that this warning is wrong in the case that the destination buffer is also dependant on the strlen of the source string.

Assume the following code:


    char my_string[strlen(argv[1] - 1)];
    strncpy(my_string, argv[1], strlen(argv[1]) - 5);
    memcpy(&my_string[strlen(argv[1]) - 5], ".7z", 4);
    printf("my string: %s\n", my_string);

This code will generate the mentioned warning, although I don't think it should. I HAVE to use strlen to achieve what I want to; this is a relatively common use case. I know I can use memcpy instead of the strncpy, but regarding that her I'm just dealing with strings I'd rather use strncpy.
Comment 3 Martin Sebor 2019-07-02 20:21:10 UTC
The warning isn't wrong -- the strncpy bound does depend on the length of the source argument.  But I agree that it isn't helpful.  As I mentioned in comment #1 there may be a way to avoid it in a subset of instances but it requires some non-trivial changes.  At a minimum these come to mind:

1) track the size of the memory allocated either by malloc or alloca/VLAs
2) look at the next statement to see if it appends the terminating nul without assuming the strncpy call itself appended it

That said, (2) already exists, albeit to a limited extent, so it needs to be extended to other places.  (1) would be useful for many other things (e.g., detection of heap buffer overflow or reading from uninitialized heap memory).  It's something I'm already planning to work on in any case, so I'll see if I can deal with this as well.
Comment 4 Rangel Moreira Fischer 2022-03-07 20:01:11 UTC
Same problem too.

static void write_my_message( char* message, size_t max_message_size, char* my_message )
{
    size_t messag_len = strlen(my_message) + 1;
        
    if( messag_len > max_message_size )
    {
        // trunc message
        strncpy(message, my_message, max_message_size - 1);  // ex: max_message_size = max buf size = 50(0 to 49). 0 to 48 = 49 elements(max_message_size - 1). 
        message[max_message_size - 1] = '\0';  // message[50-1] = message[49] = '\0'.
    }
    else
    {        
        strncpy(message, my_message, messag_len);
    }     
}


In function 'write_my_message':

error: 'strncpy' specified bound depends on the length of the source argument [-Werror=stringop-overflow=]

strncpy(message, my_message, messag_len);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

note: length computed here
size_t messag_len = strlen(my_message) + 1;
                    ^~~~~~~~~~~~~~~~~~
Comment 5 Rangel Moreira Fischer 2022-03-07 22:19:58 UTC
I am using esp-idf sdk. I think gcc version is 8.4.0.
Comment 6 Rangel Moreira Fischer 2022-03-07 22:39:51 UTC
When Compiler Optimization Level: Debug(-Og).
Compile OK.

When Compiler Optimization Level: Optimize for performance(-O2).
Compile Error.
Comment 7 Martin Sebor 2022-03-17 20:04:01 UTC
I'm no longer working on this.