Build warnings -- [-Wrestrict]

[henrygab]

Original Log

expand

/home/runner/.arduino15/packages/adafruit/hardware/samd/1.6.0/cores/arduino/WString.cpp: In member function 'void String::remove(unsigned int, unsigned int)':
/home/runner/.arduino15/packages/adafruit/hardware/samd/1.6.0/cores/arduino/WString.cpp:701:9: warning: 'char* strncpy(char*, const char*, size_t)' accessing 0 or more bytes at offsets [-1073741824, 1073741823] and [-1073741824, 1073741823] may overlap up to 4294967295 bytes at offset [5368709117, 1073741823] [-Wrestrict]
  701 |  strncpy(writeTo, buffer + index + count,len - index);
      |  ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

See https://github.com/adafruit/ArduinoCore-samd/runs/965558867

Simplified log w/hex offsets

.../samd/1.6.0/cores/arduino/WString.cpp:701:9:

warning: 'char* strncpy(char*, const char*, size_t)' accessing 0 or more bytes
    at offsets [0xC000_0000, 0x3FFF_FFFF] and [0xC000_0000, 0x3FFF_FFFF]
    may overlap up to 0xFFFF_FFFF bytes at offset [0x1_3FFF_FFFD, 0x3FFF_FFFF]
    [-Wrestrict]

701 |  strncpy(writeTo, buffer + index + count,len - index);
    |  ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

-Wrestrict information

From https://en.cppreference.com/w/c/language/restrict:

During each execution of a block in which a restricted pointer P is declared (typically each execution of a function body in which P is a function parameter), if some object that is accessible through P (directly or indirectly) is modified, by any means, then all accesses to that object (both reads and writes) in that block must occur through P (directly or indirectly), otherwise the behavior is undefined:

See GCC documentation for an example bug.

Interpretation

strncpy() marks the source and destination as restricted pointers. Therefore, the caller must detect and prevent overlap between the source and destination.

Recommendation

Preferably, rewrite to use strncpy_s() (added in C++11), which handles and reports additional errors (including overlapping src / dest).

Otherwise, write the code that validates that destination and source do not overlap, prior to calling strncpy().

[hathach]

Hi @henrygab you could just submit PR directly without filling these issue, you don't even have to explain the necessity of this -W option. We could just enable and get rid of warnings as many as we could. I don't want you to spend more time than needed on these issues, just submitted the PR, paste the compiler output ( I skip the English text anyway 😄 )

[henrygab]

I took a look at the code. It's actually a simple fix here, as the code is 100% of the time providing overlapped pointers to strncpy(). :

The code is removing a prefix from the string

ArduinoCore-samd/cores/arduino/WString.cpp

Lines 695 to 703 in 8d3b49b

	void String::remove(unsigned int index, unsigned int count){
	if (index >= len) { return; }
	if (count <= 0) { return; }
	if (count > len - index) { count = len - index; }
	char *writeTo = buffer + index;
	len = len - count;
	strncpy(writeTo, buffer + index + count,len - index);
	buffer[len] = 0;
	}

Therefore, by the time strncpy() is called, there's no error condition possible if re-implementing this to simply copy characters one at a time (a simple for loop).

Unfortunately, I only have this BSP locally via board manager at the moment. But, this is an ultra-simple fix.