Bug 371869

Summary:	support '%' in symbol Z-encoding
Product:	[Developer tools] valgrind	Reporter:	Ivo Raisr <ivosh>
Component:	general	Assignee:	Ivo Raisr <ivosh>
Status:	RESOLVED FIXED
Severity:	normal	CC:	ivosh, philippe.waroquiers
Priority:	NOR
Version:	3.12 SVN
Target Milestone:	---
Platform:	Compiled Sources
OS:	Solaris
Latest Commit:	r16112	Version Fixed In:
Attachments:	proposed patch proposed patch v2 proposed patch v3

Description Ivo Raisr 2016-10-30 15:10:48 UTC

Support '%' (percent character) in Z-encoding of function replacement and wrapping.

Solaris libc contains several implementations of memmove, memcpy, memset, based on CPU capabilities. For example for memmove, libc on x86 contains:
- memmove() - the base function
- memmove%avx2() - utilizing AVX-2 instructions when available

The point here is that the detection and correct setup (which memmove implementation to use) is done by dynamic linker. Therefore we need to replace all memmove variants, not only the base one.

However specifying "memmove" and "memmoveZa" in shared/vg_replace_strmem.c causes also other functions to be replaced (such as memmove_s() which has different number of arguments then memmove()). So we need to specify something like "memmove" and "memmove%Za". However '%' needs to be escaped from the C preprocessor.

Comment 1 Ivo Raisr 2016-10-30 16:40:49 UTC

Created attachment 101900 [details]
proposed patch

Comment 2 Philippe Waroquiers 2016-10-30 21:24:32 UTC

(In reply to Ivo Raisr from comment #1)
> Created attachment 101900 [details]
> proposed patch
Quickly read the patch, is seems strange to
have both new cases emitting % :

+         case 'P': EMITFN('%'); break;
          case 'R': EMITFN(')'); break;
+         case 'S': EMITFN('%'); break;

Comment 3 Ivo Raisr 2016-10-31 00:29:57 UTC

Created attachment 101909 [details]
proposed patch v2

Comment 4 Ivo Raisr 2016-10-31 00:30:56 UTC

Well, umm, that is of course wrong. It should have read:
+         case 'P': EMITFN('%'); break;
          case 'R': EMITFN(')'); break;
+         case 'S': EMITFN('/'); break;

Thank you for catching it! Have a look at patch #v2.

Comment 5 Ivo Raisr 2016-10-31 01:06:18 UTC

Created attachment 101910 [details]
proposed patch v3

And for memset%* replacement to work, the following needs to be changed as well:

 #define MEMSET(soname, fnname) \
-   void* VG_REPLACE_FUNCTION_EZU(20210,soname,fnname) \
+   void* VG_REPLACE_FUNCTION_EZZ(20210,soname,fnname) \
             (void *s, Int c, SizeT n); \
-   void* VG_REPLACE_FUNCTION_EZU(20210,soname,fnname) \
+   void* VG_REPLACE_FUNCTION_EZZ(20210,soname,fnname) \
             (void *s, Int c, SizeT n) \

Comment 6 Ivo Raisr 2016-11-02 21:51:12 UTC

Fixed in SVN r16112.