Skip to content

valgrind: WString: strings overlap #5949

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
6 tasks done
d-a-v opened this issue Apr 4, 2019 · 1 comment · Fixed by #5966
Closed
6 tasks done

valgrind: WString: strings overlap #5949

d-a-v opened this issue Apr 4, 2019 · 1 comment · Fixed by #5966

Comments

@d-a-v
Copy link
Collaborator

d-a-v commented Apr 4, 2019

Basic Infos

  • This issue complies with the issue POLICY doc.
  • I have read the documentation at readthedocs and the issue is not addressed there.
  • I have tested that the issue is present in current master branch (aka latest git).
  • I have searched the issue tracker for a similar issue.
  • If there is a stack dump, I have decoded it.
  • I have filled out all fields below.

Platform

  • Hardware: any
  • Core Version: git
  • Development Env: other: host emulation
  • Operating System: linux

Settings in IDE

  • Module: any
  • Flash Mode: any
  • Flash Size: any
  • lwip Variant: any
  • Reset Method: any
  • Flash Frequency: any
  • CPU Frequency: any
  • Upload Using: any
  • Upload Speed: any

Problem Description

valgrind complains with core/esp8266/WString.cpp:

Source and destination overlap in strcpy(0x1fff0000e0, 0x1fff0000e0)

MCVE Sketch

void setup ()
{
        String blah = "blah";
        blah.replace("xx","y");
}

void loop ()
{
}

How to reproduce:

cd tests/host
mkdir -p ino/WString
# put MCVE in ino/WString/WString.ino
make -j FORCE32=0 ino/WString/WString
# ...
valgrind bin/WString/WString

line 747:

strcpy(writeTo, readFrom);

@TD-er
Copy link
Contributor

TD-er commented Apr 4, 2019

It looks like the fix needs to: (first two are too obvious, but just for completeness)

  • check pointers differ
  • determine nr of bytes to move/copy
  • Do not use copy, but use memmove (see warnings at both memcpy and strcpy)

N.B. memcpy is used at more places in the same function on the same arrays.

earlephilhower added a commit to earlephilhower/Arduino that referenced this issue Apr 10, 2019
Fixes esp8266#5949

Adds a test from the issue above and fixes the problem valgrind found.
earlephilhower added a commit that referenced this issue Apr 10, 2019
* Fix String.replace overlapping strcpy

Fixes #5949

Adds a test from the issue above and fixes the problem valgrind found.

Additional pathological memcpy->memmove fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants