Bug 380397 - s390x: __GI_strcspn() replacemenet needed
Summary: s390x: __GI_strcspn() replacemenet needed
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (other bugs)
Version First Reported In: 3.13 SVN
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Ivo Raisr
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-05-31 18:14 UTC by Andreas Arnez
Modified: 2017-06-05 21:09 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments
simple patch (371 bytes, patch)
2017-05-31 20:08 UTC, Ivo Raisr
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Arnez 2017-05-31 18:14:56 UTC
With glibc 2.24, valgrind emits three "invalid read of size 1" for the program below.  This is because the strtok implementation on s390x invokes __strcspn_c(), which operates 4-byte-wise on a word-aligned string pointer.  The "invalid reads" refer to the three bytes between the 5-byte buffer and the next word boundary.
On some other platforms the strtok implementation calls strcspn via a PLT slot, in which case Valgrind's replacement for strcspn gets invoked instead.

-- >8 --
#include <string.h>

int main(int argc, char *argv[])
{
        strtok(strdup("xxxx"), "ab");

        return 0;
}
Comment 1 Ivo Raisr 2017-05-31 20:08:23 UTC
Seems like a simple wrapper for "__strcspn_c" would suffice in shared/vg_replace_strmem.c.

Would you please try the following patch (also attached) against Valgrind from SVN [1]


--- shared/vg_replace_strmem.c	(revision 16420)
+++ shared/vg_replace_strmem.c	(working copy)
@@ -1721,6 +1721,7 @@
 
 #if defined(VGO_linux)
  STRCSPN(VG_Z_LIBC_SONAME,          strcspn)
+ STRCSPN(VG_Z_LIBC_SONAME,          __strcspn_c)
 
 #elif defined(VGO_darwin)
 


[1] http://valgrind.org/downloads/repository.html
Comment 2 Ivo Raisr 2017-05-31 20:08:49 UTC
Created attachment 105808 [details]
simple patch
Comment 3 Andreas Arnez 2017-06-01 16:02:43 UTC
(In reply to Ivo Raisr from comment #1)
> Seems like a simple wrapper for "__strcspn_c" would suffice in
> shared/vg_replace_strmem.c.
> 
> Would you please try the following patch (also attached) against Valgrind
> from SVN [1]
> [...]

Confirmed, this works.  Thanks!
Instead of the s390x-specific function name "__strcspn_c", maybe we should better redirect to the common alias "__GI_strcspn".  The following patch works as well:

Index: vg_replace_strmem.c
===================================================================
--- vg_replace_strmem.c	(revision 16429)
+++ vg_replace_strmem.c	(working copy)
@@ -1721,6 +1721,7 @@
 
 #if defined(VGO_linux)
  STRCSPN(VG_Z_LIBC_SONAME,          strcspn)
+ STRCSPN(VG_Z_LIBC_SONAME,          __GI_strcspn)
 
 #elif defined(VGO_darwin)
Comment 4 Ivo Raisr 2017-06-01 20:48:11 UTC
I will integrate the change with __GI_strcspn() after Valgrind 3.13 is released.
Comment 5 Ivo Raisr 2017-06-05 21:09:22 UTC
Fixed in Valgrind SVN r16436.